[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-20 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. import-std-module test changes look good to me, thanks for fixing that up. And yes, ideally the tests should never use any libc++ internal names (and LLDB should never print them for those tests). So I think not having those in the here is a step in the right directio

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

2022-01-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I'm really sorry @martong , but I no longer work on LLDB (or the ASTImporter) and I'm not really in the loop regarding LLDB development. (Having said that, I still happy to answer questions about my own patches/reviews that I did in the past in case there are question

[PATCH] D112804: [ASTImporter] Remove ASTNodeImporter::IsStructuralMatch overload for EnumConstantDecl

2021-10-30 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG01b3bd3992b4: [ASTImporter] Remove ASTNodeImporter::IsStructuralMatch overload for… (authored by teemperor). Herald added a subscriber: cfe-commits.

[PATCH] D112796: [ASTImporter] Remove redundant IsStructuralMatch overloads

2021-10-29 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG96808c69a13c: [ASTImporter] Remove redundant IsStructuralMatch overloads (authored by teemperor). Herald added a subscriber: cfe-commits. Repository

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

2021-10-18 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D105168#3071152 , @craig.topper wrote: > In D105168#3069499 , @teemperor > wrote: > >> This broke the modules build: >> >> While building module 'LLVM_Utils' imported from >> lvm

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

2021-10-18 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This broke the modules build: While building module 'LLVM_Utils' imported from lvm/lib/Support/TargetParser.cpp:14: In file included from :209: llvm/include/llvm/Support/RISCVISAInfo.h:15:10: fatal error: could not build module 'LLVM_Option' #include "llvm/Opt

[PATCH] D102923: [clang][lex] Remark on search path usage

2021-10-12 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor reopened this revision. teemperor added a comment. This revision is now accepted and ready to land. This seems to have broken compiling some versions of Foundation/Dispatch with -fmodules enabled (which breaks the LLDB test suite when it tries to compile any Objective-C tests). Can you

[PATCH] D111387: [NFC] [Clang] Remove pre-computed complex float types

2021-10-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. Would be nice to have the motivation for this in the commit message (IIUC this is based on the suggestion from D109948

[PATCH] D108919: [clan-repl] Install clang-repl

2021-08-30 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This is essentially what D106813 was supposed to do, so this is also LGTM. You probably want to file a bugreport for @tstellar to cherry-pick this to the release. The biggest impact from this seems to be that it increases the release

[PATCH] D108407: [CodeGen][WIP] Avoid generating Record layouts for pointee types

2021-08-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I'm mostly putting this up to get some early feedback if anyone sees a problem with using opaque types here (e.g. it breaks some optimizations, etc.). If it does, it would still be nice if we could at least make this happen on some opt-in bases as it would be very ben

[PATCH] D108407: [CodeGen][WIP] Avoid generating Record layouts for pointee types

2021-08-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor created this revision. teemperor added reviewers: dblaikie, rjmccall, rsmith, v.g.vassilev. teemperor added a project: clang. Herald added a subscriber: dexonsmith. teemperor requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits. Herald added a project: LL

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor updated this revision to Diff 364385. teemperor added a comment. - Make setter propagate 'deserialize' to the node dumper (thanks Volodymyr!) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80878/new/ https://reviews.llvm.org/D80878 Files: clang/include/clang/AST/ASTNodeTrave

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2021-08-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This is technically in the "Waiting on Review" queue, that's why I also added another reviewer :). The original revision got accepted and then I reverted and updated the patch/test to honor the `Deserialize` value (which was broken in the first version). I guess the f

[PATCH] D106813: [clang-repl] Build and install clang-repl by default

2021-07-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. I think my concerns from D96033 are all addressed by now, so this LGTM. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D106266: [C++4OpenCL] Add run line standard aliases clc++1.0 and CLC++1.0

2021-07-22 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Quick note to prevent some confusion: I saw this patch and realized that the LLDB change was only necessary because some Clang code got copy pasted into LLDB. I removed that copy in https://reviews.llvm.org/D106537 so if you see merge conflicts while merging this, you

[PATCH] D105926: [PowerPC] Extra test case for LDARX

2021-07-17 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Sorry for raising an unrelated topic here, but I can't reach @Conanap directly via the mail from the git commits: @Conanap could you please create the git branches for your patches in your own Github fork instead of the main LLVM repo? LLVM's policy is to have working

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

2021-07-15 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I'm really sorry, I don't understand what kind of review is expected here given the state of this patch. - The patch **still** doesn't include the tests that I asked for in my

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

2021-07-13 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I guess Phabricator just sent this back to review automatically when you updated the diff? Just mark this as 'needs review' again when this is ready for review. I'll send this

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

2021-07-12 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I think this looks good, thanks for fixing this! I believe there should be a few more tests for all the corner cases we can run into here (those tests can all be just Python AP

[PATCH] D104918: [clang-repl] Implement partial translation units and error recovery.

2021-07-12 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. @v.g.vassilev For LLDB you need to change the `TypeSystemClang::SetExternalSource` function to this (it just moves the `setHasExternalLexicalStorage` one line up. You can just add that change and the compilation fix to this commit. void TypeSystemClang::SetExternal

[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

2021-06-11 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:164 + const auto *MethodDecl = dyn_cast_or_null(Call.getDecl()); + if (!MethodDecl || !MethodDecl->getParent()) +return {}; `getParent()` will assert when th

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

2021-06-11 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D103750#2812613 , @RedDocMD wrote: > How do I set the C++ standard while running a test? If I understand your language correctly, then see my inline comment. Comment at: clang/test/Analysis/smart-ptr-text

[PATCH] D102903: [clang] Fix Wnested-anon-types in ABIArgInfo

2021-05-21 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGebd25fde5e04: [clang] Fix Wnested-anon-types in ABIArgInfo (authored by teemperor). Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-05-21 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:243 + // If b is a get() expression, then we can return a note + if (auto Report = bugReportOnGet(Node, BRC, RHS)) { +return Report;

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. (Tabbed on the Submit button instead of finishing that quote block...) So I assume the stage2- targets are just invoking some ninja invocations in sequence? Anyway, what I think it would be helpful to see what link jobs were in progress. But I guess even with 32 jobs

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D96033#2768940 , @phosek wrote: > In D96033#2767884 , @teemperor wrote: > >> In D96033#2766502 , @phosek wrote: >> >>> In D96033#2766372

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D96033#2766502 , @phosek wrote: > In D96033#2766372 , @v.g.vassilev > wrote: > >> In D96033#2766332 , @phosek wrote: >> >>> We've started seein

[PATCH] D99946: [DebugInfo] Fix DWARF expressions for __block vars that are not on the heap

2021-05-17 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG888ce70af288: [DebugInfo] Fix DWARF expressions for __block vars that are not on the heap (authored by teemperor). Herald added a subscriber: cfe-com

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-05-06 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Just to be clear, I am not sure when/if D101950 is ready. It's mostly blocked by me not having the time to work on it, so depending how my schedule is this might land soon or far in the future. Just linked it here as this (temporary)

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-05-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. By the way, I set up a WIP patch for changing LLDB's import away from the current MinimalImport of records to something that is more in line with how Clang works: https://reviews.llvm.org/D101950 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. (Obviously this should still wait on Richard & John as I think they still have unaddressed objections) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96033/new/ https://reviews.llvm.org/D96033 ___ cfe-commits mailin

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. I believe everything I pointed out so far has been resolved. I still have one more nit about the unit test though (just to not make it fail on some Windows setups). FWIW, given that thi

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-04-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Sorry for the delay. I think all my points have been resolved beside the insertion SourceLoc. Comment at: clang/lib/Interpreter/IncrementalParser.cpp:184 + SourceLocation NewLoc = SM.getLocForStartOfFile(SM.getMainFileID()) +

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

2021-04-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Not sure what the backtrace is for the `clang::FieldDecl::isZeroSize` crash but what might relevant: 1. The ASTImporter test isn't using the 'Minimal' import mode that LLDB is using. In the tests we are importing all declarations directly. In LLDB we use the 'minimal

[PATCH] D99740: Avoid calling ParseCommandLineOptions in BackendUtil if possible

2021-04-01 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG60854c328d87: Avoid calling ParseCommandLineOptions in BackendUtil if possible (authored by teemperor). Herald added a subscriber: cfe-commits. Repo

[PATCH] D99512: [ObjC][CodeGen] Fix missing debug info in situations where an instance and class property have the same identifier

2021-03-30 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1cbba533ec93: [ObjC][CodeGen] Fix missing debug info in situations where an instance and… (authored by teemperor). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D99162: [ASTImporter] Split out Objective-C related unit tests

2021-03-23 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG274907c0a4d6: [ASTImporter] Split out Objective-C related unit tests (authored by teemperor). Herald added a subscriber: cfe-commits. Repository:

[PATCH] D99077: [ASTImporter] Fix import of ObjCPropertyDecl that share the same name

2021-03-22 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe421a74108ee: [ASTImporter] Fix import of ObjCPropertyDecl that share the same name (authored by teemperor). Herald added a subscriber: cfe-commits.

[PATCH] D98951: [clang][ASTImporter] Add import API for 'const Type *' (NFC).

2021-03-19 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Herald added a subscriber: whisperity. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98951/new/ https://reviews.llvm.org/D98

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. It seems we already hit this issue before and decided to add a workaround: https://reviews.llvm.org/D86823 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97850/new/ https://reviews.llvm.org/D97850 ___

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

2021-03-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Sorry for the delay. Yes, I meant the `--vg` flag which is I think the only valid way to test this from your description. That it can't be deterministically tested without this (or in configurations that don't run valgrind) is fine IMHO. Repository: rG LLVM Github

[PATCH] D97850: Fix PCM read from ModuleCache for ext4 filesystem

2021-03-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. If I understand this correctly, then the inode caching logic in FileManager isn't just breaking PCMs but all file operations: TEST_F(FileManagerTest, InodeReuse) { { std::ofstream myfile; myfile.open("a.cpp"); myfile << "content\n"; } ll

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

2021-03-03 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. We can (and do) run LIT tests under valgrind, so if you have a test that triggers this valgrind error then that seems like the way to go. I don't think the test has to deterministically fail outside valgrind to be a valid test. Repository: rG LLVM Github Monorepo

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

2021-02-25 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa54f160b3a98: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk (authored by haampie, committed by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-02-17 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added inline comments. Comment at: clang/lib/Interpreter/IncrementalParser.cpp:184 + SourceLocation NewLoc = SM.getLocForStartOfFile(SM.getMainFileID()) + .getLocWithOffset(InputCount + 2); + The `+ 2` here is probably not

[PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

2021-02-16 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. LGTM. Regarding the LLDB example: Given that the LLDB API is in theory not bound to the semantics a specific language, I think one can argue that `IsAnonymousType` could also return true for unnamed classes. The use case that triggered this whole discussion was someo

[PATCH] D96427: Support multi-configuration generators correctly in several config files

2021-02-11 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D96427#2555251 , @JDevlieghere wrote: > LGTM. I'm surprised the `dsymutil` one slipped through the cracks, we have a > bot that should (?) be testing this configuration: > http://green.lab.llvm.org/green/view/LLDB/job/lldb-

[PATCH] D96049: [Timer] On macOS count number of executed instructions

2021-02-11 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7e3b9aba609f: [Timer] On macOS count number of executed instructions (authored by ahoppen, committed by teemperor). Repository: rG LLVM Github Mon

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-02-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a reviewer: sgraenitz. teemperor added a comment. Another round of comments. Also a general note that applies to a bunch of places, if you specify the function names in comments when calling a function, you have to add a trailing `=` so that clang-tidy can actually check this fo

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-02-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. A more general comment: You have to `std::move` your `llvm::Error` variables when the result is `llvm::Expected` (otherwise this won't compile). Comment at: clang/include/clang/Interpreter/Transaction.h:1 +//===--- Transaction.h - Incremental Compila

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

2020-12-14 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I believe rsmith is in GMT-8, so I assume this won't get a fix soon and I went ahead and reverted in 22ccdb787024e954318e35fcf904fd4fa36f5679 Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

2020-12-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. There are several LLDB tests failing with this change. The TestImportBaseClassWhenClassHasDerivedMember.py is probably the only relevant one. The others are probably failing for the same

[PATCH] D92209: [ASTImporter] Support CXXDeductionGuideDecl with local typedef

2020-12-07 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D92209#2437416 , @martong wrote: > Ping. > @teemperor please take a look if you have some time. This is a really > important patch which may influence some future directions regarding the > ASTImporter. Apologies, last week

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

2020-12-04 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Having the comparison code would be nice. The custom types in the associated types aren't compared by the generic code, so an overload like this in the `StmtComparer` class should be enough: bool IsStmtEquivalent(const GenericSelectionExpr *E1, const GenericSelecti

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-02 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D92103#2428139 , @martong wrote: > Hey Raphael, thanks for looking into the CTU crash! > > I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC > 7. I think the discrepancy stems from GCC's libstdc++ an

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-01 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. @gamesh411 I recreated the setup you listed (thanks for that btw) but for me this works just fine. I assume the crash happens in a class template from one of the external libraries. It probably works for me because I don't have the same library version as you have, bu

[PATCH] D92106: [ASTImporter] Import the default argument of NonTypeTemplateParmDecl

2020-11-27 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG89c1a7a67d69: [ASTImporter] Import the default argument of NonTypeTemplateParmDecl (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added inline comments. Herald added a subscriber: JDevlieghere. Comment at: clang/lib/AST/ASTImporter.cpp:5161 - // FIXME: Import default argument and constraint expression. + // FIXME: Import constraint expression. martong wrote: > I wonder whet

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3f6c856bb5ae: [ASTImporter] Import the default argument of TemplateTypeParmDecl (authored by teemperor). Herald added projects: clang, LLDB. Herald added subscribers: lldb-commits, cfe-commits. Changed pr

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. I think the test could be made a bit stricter (see inline comment), but otherwise this LGTM. Thanks! Comment at: clang/unittests/AST/ASTImporterTest.cpp:5900 + ASSERT

[PATCH] D92119: [ASTImporter] Import the default argument of TemplateTemplateParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG39a5dd164ca8: [ASTImporter] Import the default argument of TemplateTemplateParmDecl (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Gi

[PATCH] D92109: [ASTImporter] Support import of CXXDeductionGuideDecl

2020-11-25 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. You think it's worth it to have a test for a user-specified deduction guide? I think this is missing an 'explicit' deduction guide test, so maybe we could test both things in a

[PATCH] D92016: [ASTImporter] Make the Import() return value consistent with the map of imported decls when merging ClassTemplateSpecializationDecls

2020-11-24 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0c926e6d245b: [ASTImporter] Make the Import() return value consistent with the map of… (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM

[PATCH] D83038: [clang] Add an option for hiding line numbers in diagnostics

2020-11-05 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG659f4bd87efc: [clang] Add an option for hiding line numbers in diagnostics (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Mono

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2020-10-29 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor updated this revision to Diff 301577. teemperor added a comment. - Respect forced deserialisation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80878/new/ https://reviews.llvm.org/D80878 Files: clang/include/clang/AST/TextNodeDumper.h clang/lib/AST/ASTDumper.cpp clang/

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

2020-10-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. >> The tests are just loading a Clang module and then trying to import some >> Decls into another ASTContext, so I don't think the error here is specific >> to LLDB. We just don't have any ASTImporter tests that exercise modules. > > I'll add one. I'm actually not eve

[PATCH] D89749: SourceManager: Don't allocate an SLocEntry until it's loaded

2020-10-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a subscriber: v.g.vassilev. teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Maybe arc didn't correctly apply the patch, but this causes a few LLDB tests to fail on my machine: lldb-api :: commands/expressi

[PATCH] D89748: SourceManager: Use the same fake SLocEntry whenever it fails to load

2020-10-26 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89748/new/ https://reviews.llvm.org/D89748 ___ cfe-commits mailing list cfe-comm

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

2020-10-13 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp:2 +int foo(const char * fmt, ...) +__attribute__ ((__format__ (__scanf__, 1, 2)));

[PATCH] D88984: Prefer libc++ headers in the -isysroot over the toolchain

2020-10-12 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. From the LLDB side we can just change our test setup to explicitly add an include the libc++ in our build tree (and have -nostdinc to avoid the libc++ in the SDK). So it wouldn't be a problem for us. I can make a review for that once this gets accepted. Repository:

[PATCH] D88780: Allow interfaces to operate on in-memory buffers with no source location info.

2020-10-09 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I think we could at least have a unittest that just calls these functions with an invalid SourceLocation. `unittests/Basic/SourceManagerTest.cpp` already takes care of creating a valid SourceManager object, so the unit test would just be a single call to each method.

[PATCH] D88984: Prefer libc++ headers in the -isysroot over the toolchain

2020-10-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. On macOS, LLDB's tests pass `-isysroot /path/to/SDK` to Clang but expect that Clang always prefers the the libc++ headers that are in the Clang build directory. So if we change this behaviour then we should have a way to keep the old behaviour around. Repository:

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. > I am not sure how much do you guys value these diags in LLDB I think we don't even display them most of the time (IIUC they go to the diagnostic engine which is not always hooked up for the ASTs in LLDB as only a few actually come from Clang). Repository: rG LLV

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88665/new/ https://reviews.llvm.org/D88665 __

[PATCH] D88665: [ASTImporter][AST] Fix structural equivalency crash on dependent FieldDecl

2020-10-02 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Shouldn't this compare the actual width expressions? In other words, this patch wouldn't pass the test below: TEST_F(StructuralEquivalenceTemplateTest, DependentFieldDeclDiff

[PATCH] D87619: [ASTImporter] Refactor IsStructurallyEquivalent's Decl overloads to be more consistent

2020-09-21 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7c4575e15f06: [ASTImporter] Refactor IsStructurallyEquivalent's Decl overloads to be more… (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D87444: [ASTImporter] Add basic support for comparing Stmts and compare function bodies

2020-09-13 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc0bcd11068fc: [ASTImporter] Add basic support for comparing Stmts and compare function bodies (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository:

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2020-09-07 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor reopened this revision. teemperor added a comment. This revision is now accepted and ready to land. In D80878#2258942 , @riccibruno wrote: > And what if deserialization is forced? That's a good point. That should indeed continue to work with the

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

2020-09-07 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0478720157f6: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further… (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

2020-09-02 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Just to also jump in on the thread: This broke the modules builds. I fixed that in https://reviews.llvm.org/rGe0e7eb2e2648aee83caf2ecfe2972ce2f653d306 so please recommit that patch or merge it into this revision before relanding. Thanks! Repository: rG LLVM Github

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-24 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D86308#2229936 , @tambre wrote: > In D86308#2229901 , @teemperor wrote: > >> Sorry, just got around to check this out. With the new workaround this seems >> to work on macOS (the initi

[PATCH] D84013: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)

2020-08-24 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG105151ca5669: Reland "Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)" (authored by teemperor). Changed prior to commit: https://reviews.llvm.org/D84013?vs=287318&id=287358#toc Reposi

[PATCH] D84013: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)

2020-08-24 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D84013#2233242 , @davezarzycki wrote: > I have an auto-bisecting cron job that has identified the "reland" as > breaking the test suite on Fedora 32 (x86). Is there a quick fix or can we > revert the reland? > > FAIL: Cla

[PATCH] D84013: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)

2020-08-24 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGada2e8ea6739: Reland "Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)" (authored by teemperor). Changed prior to commit: https://reviews.llvm.org/D84013?vs=287017&id=287318#toc Reposi

[PATCH] D84013: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)

2020-08-21 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor reopened this revision. teemperor added a comment. This revision is now accepted and ready to land. Somehow this ended up failing on Fuchsia with a plain "Exit code 1", but not sure what's the fault. I'll revert until I figured that out. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D84013: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347)

2020-08-21 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa4c3ed42ba56: Correctly emit dwoIDs after ASTFileSignature refactoring (D81347) (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

2020-08-21 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. Sorry, just got around to check this out. With the new workaround this seems to work on macOS (the initial patch did produce the same error). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

2020-08-20 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I reverted in adf0b8cc70325f027d202139e3ff984c41896b57 to get the bots green again . Feel free to ping if you need someone to test the patch on macOS. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

2020-08-20 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This broke Green Dragon's compiler-rt builds and also my local compiler-rt build. See for example here: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/23424/console Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D84829: [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit ObjCInterfaceDecl

2020-08-11 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG02899d7f1b9a: [clang] Don't make ObjCIvarDecl visible twice when adding them to an implicit… (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: r

[PATCH] D84828: [clang] Don't make synthesized accessor stub functions visible twice

2020-08-11 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG442a80292d50: [clang] Don't make synthesized accessor stub functions visible twice (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Git

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-08 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM, thanks for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83008/new/ https://reviews.llvm.org/D83008 ___ cfe-commi

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-07 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. I think we are talking about different things. My question was why is this a 'Shell' test (like, a test in the `Shell` directory that uses FileCheck) and not a test in the `API` directory (using Python). An 'API' test could use the proper expression testing tools. And

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

2020-07-06 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Fixed here to get the bot running again: https://github.com/llvm/llvm-project/commit/7308e1432624f02d4e652ffa70e40d0eaa89fdb3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82585/new/ https://reviews.llvm.org/D82585 __

[PATCH] D82585: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

2020-07-06 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This broke the Green Dragon build for Clang: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/ FAILED: bin/diagtool : && /Users/buildslave/jenkins/workspace/lldb-cmake/host-compiler/bin/clang++ -Wdocumentation -fPIC -fvisibility-inlines-hidden -Werror=dat

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-06 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. In D83008#2131796 , @tschuett wrote: > You could try: > > clangxx_host -Xclang -fdump-record-layouts %p/Inputs/layout.cpp -emit-llvm > -o /dev/null > > > You could commit the record layouts without your fix to show the differe

[PATCH] D83008: Fix ItaniumRecordLayoutBuilder so that is grabs the correct bases class offsets from the external source

2020-07-05 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Thanks for tracking this down, this is a really nasty bug... The fix itself is obviously fine, but I think I'm out of the loop regarding the testing strategy. We talked about a

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions

2020-06-09 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. This introduced a compiler warning: /Users/teemperor/1llvm/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1102:23: warning: 'getDecl' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] cons

[PATCH] D78086: [ASTImporter] Also import overwritten file buffers

2020-04-27 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9f1e81f1c0ac: [ASTImporter] Also import overwritten file buffers (authored by teemperor). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

2020-04-20 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. Beside Gabors comment I think I'm happy with this. Thanks! Comment at: clang/unittests/AST/ASTImporterTest.cpp:5939 + +/// An ExternalASTSource that keeps track of the tags is completed. +struct SourceWithCompletedTagList : clang::ExternalASTSource {

  1   2   3   4   >