https://github.com/JDevlieghere edited
https://github.com/llvm/llvm-project/pull/94208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/94208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -72,7 +72,7 @@ class PrintFunctionsConsumer : public ASTConsumer {
*sema.LateParsedTemplateMap.find(FD)->second;
sema.LateTemplateParser(sema.OpaqueParser, LPT);
llvm::errs() << "late-parsed-decl: \"" << FD->getNameAsString() <<
"\"\n";
-}
+
https://github.com/JDevlieghere approved this pull request.
🥳
https://github.com/llvm/llvm-project/pull/95164
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere edited
https://github.com/llvm/llvm-project/pull/95164
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JDevlieghere wrote:
`LLDB_CURSES_LIBS` is scoped to lldbCore, which the EditLine unit test isn't
linking. If its also necessary in Host, then we should hoist it up and link
both libraries against it.
Anyway, that also means my initial assessment was wrong. I looked for the
header include but
JDevlieghere wrote:
This is awesome, thank you for working on this!
https://github.com/llvm/llvm-project/pull/94208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JDevlieghere wrote:
+1 to what Adrian said. Taht certainly does **not** match my expectations. I'm
not sure why there would be a different policy for LLDB than for any of the
other monorepo projects.
https://github.com/llvm/llvm-project/pull/94208
__
JDevlieghere wrote:
Hey @etcwilde, this breaks the Xcode standalone build for lldb:
https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-standalone/121/
The failing command in the generated script phase is:
```
cmake -E copy_directory
/Users/jonas/llvm/xcode-build/Debug/bin/LLD
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/89260
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere approved this pull request.
LLDB & VFS change LGTM.
https://github.com/llvm/llvm-project/pull/89140
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/89140
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere requested changes to this pull request.
+1 on what Cyndy said. The lldb change itself looks fine.
https://github.com/llvm/llvm-project/pull/89480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llv
JDevlieghere wrote:
This looks like a nice improvement for folks using those generators. Even
though most of these changes look straightforward, it would be a lot easier to
review if this was broken up per subproject. Is there any reason that's not
possible?
https://github.com/llvm/llvm-proj
JDevlieghere wrote:
Nice. An added benefit of doing it this way is that you can always set the
property for a particular target if you really need the scheme and you can do
it in the same cmake file that creates the target (i.e. in a subproject).
https://github.com/llvm/llvm-project/pull/1012
https://github.com/JDevlieghere approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/101243
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/99871
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/99871
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere requested changes to this pull request.
We should be using `-platform-version` for newer versions of the linker. How
are we ending up in this code path?
https://github.com/llvm/llvm-project/pull/88810
___
cfe-commits m
https://github.com/JDevlieghere edited
https://github.com/llvm/llvm-project/pull/88810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JDevlieghere wrote:
Right, my point is that (I think) that doesn't make sense. AFAIK
`addMinVersionArgs` is called exclusively from the code snippet I pasted, which
should check the linker version. The version of `ld` in which this changed is
much newer than the 520 referenced there.
I'd sta
JDevlieghere wrote:
> If this is split out from the other larger PR, should there be `clang/`
> changes in here?
+1, please unstage the `clang` and `openmp` changes.
https://github.com/llvm/llvm-project/pull/91858
___
cfe-commits mailing list
cfe-co
https://github.com/JDevlieghere requested changes to this pull request.
As per Aiden's suggestion, please split this up into smaller PRs, grouped by
subproject.
https://github.com/llvm/llvm-project/pull/91856
___
cfe-commits mailing list
cfe-commits@
JDevlieghere wrote:
I don't think Alex is arguing in favor of keeping the old (wrong) behavior, but
the first file looks like this:
```
foundSpec = False
if [...]
foundSpec = True
[...]
if foundSpec is False:
```
It's pretty obvious this is a boolean and should use `if not foundSpec`.
http
https://github.com/JDevlieghere commented:
This should really be broken up into separate PRs per subproject. One large PR
like this makes reviewing harder and causes unnecessary churn in the case that
this gets reverted.
https://github.com/llvm/llvm-project/pull/91857
_
JDevlieghere wrote:
> It's a `sed s/== None/is None/g` - what is there to review? 10 separate
> commits/PRs for the same exact `sed` costs more in commit noise (and effort
> on the part of @e-kwsm) than one solid, patient, review here.
In addition to what @ftynse said above, the `sed` might no
JDevlieghere wrote:
No objections in the context of LLDB. We don't use terminfo directly (although
I think editline does, but that isn't affected by this) and if we want the TUI
we depend on curses anyway.
https://github.com/llvm/llvm-project/pull/92865
__
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/92953
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JDevlieghere wrote:
The LLDB bots are still failing:
https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/as-lldb-cmake/4342/
https://ci.swift.org/view/all/job/llvm.org/view/LLDB/job/lldb-cmake/2244/
https://github.com/llvm/llvm-project/pull/92885
___
JDevlieghere wrote:
What I said was that, to the best of my knowledge, we don't have places in LLDB
that rely on `terminfo` but not on `ncruses`. I think that statement still
holds true. The assumption behind it was that `ncurses` provides a superset of
the functionality provided by `terminfo`
JDevlieghere wrote:
> Thanks @JDevlieghere! Could you add this special case?
I'm happy to help but I wouldn't feel comfortable making such a change without
being able to reproduce and test it.
> The authoritatively established and supported way to link with `ncurses` on
> most Linux platfor
Author: Jonas Devlieghere
Date: 2021-12-06T09:34:53-08:00
New Revision: 4cb79294e8df8c91ae15264d1014361815d34a53
URL:
https://github.com/llvm/llvm-project/commit/4cb79294e8df8c91ae15264d1014361815d34a53
DIFF:
https://github.com/llvm/llvm-project/commit/4cb79294e8df8c91ae15264d1014361815d34a53.d
Author: Jonas Devlieghere
Date: 2021-04-20T20:42:50-07:00
New Revision: 05eeed9691aeb3e0316712195b998e9078cdceb0
URL:
https://github.com/llvm/llvm-project/commit/05eeed9691aeb3e0316712195b998e9078cdceb0
DIFF:
https://github.com/llvm/llvm-project/commit/05eeed9691aeb3e0316712195b998e9078cdceb0.d
Author: Jonas Devlieghere
Date: 2021-04-21T14:22:13-07:00
New Revision: 6331680ad2ad000fdaf7e72f3c1880c7908ffa25
URL:
https://github.com/llvm/llvm-project/commit/6331680ad2ad000fdaf7e72f3c1880c7908ffa25
DIFF:
https://github.com/llvm/llvm-project/commit/6331680ad2ad000fdaf7e72f3c1880c7908ffa25.d
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/96385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/JDevlieghere approved this pull request.
https://github.com/llvm/llvm-project/pull/96532
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JDevlieghere wrote:
The change itself seems fine but I'm not super convinced that the motivation is
worth the churn. All these files live under the "Option" library and therefore
it's (IMHO) clear that Opt is short for Option.
Is this addressing code review feedback or a real issue where the
Timm =?utf-8?q?Bäder?=
Message-ID:
In-Reply-To:
https://github.com/JDevlieghere approved this pull request.
LGTM but please give @Michael137 a chance to take a look as this is his area of
expertise.
https://github.com/llvm/llvm-project/pull/122289
___
101 - 138 of 138 matches
Mail list logo