Re: [Lldb-commits] Prevent type canonization when dereferencing

2021-06-02 Thread RaphaelTeemperorIsemann via lldb-commits
Thanks1 fixed here: https://reviews.llvm.org/D103532

> On 2 Jun 2021, at 18:06, Andy Yankovsky  wrote:
> 
> I think the problem is specific to references, at least that's the case I ran 
> into in lldb-eval -- 
> https://github.com/google/lldb-eval/blob/master/docs/lldb-bugs.md#dereferencing-a-value-canonizes-the-type
>  
> 
> On Wed, 2 Jun 2021 at 17:26, Raphael “Teemperor” Isemann  > wrote:
> I assume your bug is for dereferencing references? In your test taking the 
> ref variable and then dereferencing it via the SB API reproduces that I think 
> you're running into:
> 
> >>> lldb.frame.FindVariable("p_ref").GetType().GetName()
> 'TPair &'
> >>> lldb.frame.FindVariable("p_ref").Dereference().GetType().GetName()
> 'TTuple'
> 
> (I made `p_ref` a local to avoid the expression evaluation machinery)
> 
> Cheers,
> - Raphael
> 
> > On 2 Jun 2021, at 15:27, Lasse Folger  > > wrote:
> > 
> > Hi Raphael,
> > 
> > I have a very similar test for a tool that integrates with lldb which 
> > failed without the patch.
> > I thought the test in the patch would behave the same which is apparently 
> > not the case.
> > Thanks for pointing that out. I will need to take another look and will get 
> > back to you once I figure out what's wrong.
> > Sorry for the inconvenience.
> > 
> > kind regards,
> > Lasse
> > 
> > 
> > On Wed, Jun 2, 2021 at 1:15 PM Raphael “Teemperor” Isemann 
> > mailto:teempe...@gmail.com>> wrote:
> > Hi Lasse,
> > 
> > the test from the patch passes for me even without your non-test changes. 
> > Not sure if you attached the wrong diff or it needs to be applied on a 
> > specific commit that is not ToT? Can you maybe try pushing your code to 
> > some git repo?
> > 
> > Your change to TypeSystemClang (which I assume removes the canonicalization 
> > of parent_qual_type) is from what I can see not actually changing the 
> > result value of `GetChildCompilerTypeAtIndex`. It looks like the return 
> > value for pointer types is computed independently from `parent_qual_type` 
> > without any canonicalization.
> > 
> > Cheers,
> > - Raphael
> > 
> >> On 2 Jun 2021, at 11:39, Lasse Folger via lldb-commits 
> >> mailto:lldb-commits@lists.llvm.org>> wrote:
> >> 
> >> <0001-lldb-prevent-canonization-of-type-when-dereferencing.patch>
> > 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] Prevent type canonization when dereferencing

2021-06-02 Thread RaphaelTeemperorIsemann via lldb-commits
I assume your bug is for dereferencing references? In your test taking the ref 
variable and then dereferencing it via the SB API reproduces that I think 
you're running into:

>>> lldb.frame.FindVariable("p_ref").GetType().GetName()
'TPair &'
>>> lldb.frame.FindVariable("p_ref").Dereference().GetType().GetName()
'TTuple'

(I made `p_ref` a local to avoid the expression evaluation machinery)

Cheers,
- Raphael

> On 2 Jun 2021, at 15:27, Lasse Folger  wrote:
> 
> Hi Raphael,
> 
> I have a very similar test for a tool that integrates with lldb which failed 
> without the patch.
> I thought the test in the patch would behave the same which is apparently not 
> the case.
> Thanks for pointing that out. I will need to take another look and will get 
> back to you once I figure out what's wrong.
> Sorry for the inconvenience.
> 
> kind regards,
> Lasse
> 
> 
> On Wed, Jun 2, 2021 at 1:15 PM Raphael “Teemperor” Isemann 
>  wrote:
> Hi Lasse,
> 
> the test from the patch passes for me even without your non-test changes. Not 
> sure if you attached the wrong diff or it needs to be applied on a specific 
> commit that is not ToT? Can you maybe try pushing your code to some git repo?
> 
> Your change to TypeSystemClang (which I assume removes the canonicalization 
> of parent_qual_type) is from what I can see not actually changing the result 
> value of `GetChildCompilerTypeAtIndex`. It looks like the return value for 
> pointer types is computed independently from `parent_qual_type` without any 
> canonicalization.
> 
> Cheers,
> - Raphael
> 
>> On 2 Jun 2021, at 11:39, Lasse Folger via lldb-commits 
>>  wrote:
>> 
>> <0001-lldb-prevent-canonization-of-type-when-dereferencing.patch>
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] Prevent type canonization when dereferencing

2021-06-02 Thread RaphaelTeemperorIsemann via lldb-commits
Hi Lasse,

the test from the patch passes for me even without your non-test changes. Not 
sure if you attached the wrong diff or it needs to be applied on a specific 
commit that is not ToT? Can you maybe try pushing your code to some git repo?

Your change to TypeSystemClang (which I assume removes the canonicalization of 
parent_qual_type) is from what I can see not actually changing the result value 
of `GetChildCompilerTypeAtIndex`. It looks like the return value for pointer 
types is computed independently from `parent_qual_type` without any 
canonicalization.

Cheers,
- Raphael

> On 2 Jun 2021, at 11:39, Lasse Folger via lldb-commits 
>  wrote:
> 
> <0001-lldb-prevent-canonization-of-type-when-dereferencing.patch>

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 4b074b4 - [lldb] Fix UB in half2float and add some more tests.

2021-05-20 Thread RaphaelTeemperorIsemann via lldb-commits
I think what you're looking for is llvm::bit_cast (in ADT/bit.h) :) But that 
code was written before that, so that's why it's still using unions

> On 19 May 2021, at 23:27, Shafik Yaghmour  wrote:
> 
> *sigh*I wish we had std::bit_cast instead of using union based type punning, 
> we do have __builtin_bit_cast but it is different enough that replacing it 
> later on w/ std::bit_cast wouldn’t just be search/replace.
> 
>> On May 19, 2021, at 12:37 PM, Raphael Isemann via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2021-05-19T21:37:10+02:00
>> New Revision: 4b074b49be206306330076b9fa40632ef1960823
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/4b074b49be206306330076b9fa40632ef1960823
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/4b074b49be206306330076b9fa40632ef1960823.diff
>> 
>> LOG: [lldb] Fix UB in half2float and add some more tests.
>> 
>> The added DumpDataExtractorTest uncovered that this is lshifting a negative
>> integer which upsets ubsan and breaks the sanitizer bot. This patch just
>> changes the variable we shift to be unsigned and adds a bunch of tests to 
>> make
>> sure this function does what it promises.
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Core/DumpDataExtractor.cpp
>>   lldb/unittests/Core/DumpDataExtractorTest.cpp
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
>> b/lldb/source/Core/DumpDataExtractor.cpp
>> index ec44e3481c1e5..34c9353c9feaa 100644
>> --- a/lldb/source/Core/DumpDataExtractor.cpp
>> +++ b/lldb/source/Core/DumpDataExtractor.cpp
>> @@ -52,7 +52,9 @@ static float half2float(uint16_t half) {
>>float f;
>>uint32_t u;
>>  } u;
>> -  int32_t v = (int16_t)half;
>> +  // Sign extend to 4 byte.
>> +  int32_t sign_extended = static_cast(half);
>> +  uint32_t v = static_cast(sign_extended);
>> 
>>  if (0 == (v & 0x7c00)) {
>>u.u = v & 0x80007FFFU;
>> 
>> diff  --git a/lldb/unittests/Core/DumpDataExtractorTest.cpp 
>> b/lldb/unittests/Core/DumpDataExtractorTest.cpp
>> index c4ec5f2e9a35b..05cd13add1e99 100644
>> --- a/lldb/unittests/Core/DumpDataExtractorTest.cpp
>> +++ b/lldb/unittests/Core/DumpDataExtractorTest.cpp
>> @@ -174,8 +174,30 @@ TEST(DumpDataExtractorTest, Formats) {
>>   "{0x 0x}");
>> 
>>  // See half2float for format details.
>> +  // Test zeroes.
>> +  TestDump(std::vector{0x, 0x8000},
>> +   lldb::Format::eFormatVectorOfFloat16, "{0 -0}");
>> +  // Some subnormal numbers.
>> +  TestDump(std::vector{0x0001, 0x8001},
>> +   lldb::Format::eFormatVectorOfFloat16, "{5.96046e-08 
>> -5.96046e-08}");
>> +  // A full mantisse and empty expontent.
>> +  TestDump(std::vector{0x83ff, 0x03ff},
>> +   lldb::Format::eFormatVectorOfFloat16, "{-6.09756e-05 
>> 6.09756e-05}");
>> +  // Some normal numbers.
>> +  TestDump(std::vector{0b011001001000},
>> +   lldb::Format::eFormatVectorOfFloat16, "{3.14062}");
>>  TestDump(std::vector{0xabcd, 0x1234},
>>   lldb::Format::eFormatVectorOfFloat16, "{-0.0609436 0.000757217}");
>> +  // Largest and smallest normal number.
>> +  TestDump(std::vector{0x0400, 0x7bff},
>> +   lldb::Format::eFormatVectorOfFloat16, "{6.10352e-05 65504}");
>> +  // quiet/signaling NaNs.
>> +  TestDump(std::vector{0x, 0xffc0, 0x7fff, 0x7fc0},
>> +   lldb::Format::eFormatVectorOfFloat16, "{nan nan nan nan}");
>> +  // +/-Inf.
>> +  TestDump(std::vector{0xfc00, 0x7c00},
>> +   lldb::Format::eFormatVectorOfFloat16, "{-inf inf}");
>> +
>>  TestDump(std::vector{std::numeric_limits::min(),
>>  std::numeric_limits::max()},
>>   lldb::Format::eFormatVectorOfFloat32, "{1.17549e-38 3.40282e+38}");
>> 
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D92759: [lldb] Introduce separate scratch ASTs for debug info types and types imported from C++ modules.

2021-01-25 Thread RaphaelTeemperorIsemann via lldb-commits
Yeah, that's an unfortunate bug in GCC 5.x. I actually just fixed that before 
the weekend in 
https://reviews.llvm.org/rG37510f69b4cb8d76064f108d57bebe95984a23ae 
 

+CC Tom (who is the release manager AFAIK), as that commit probably deserves to 
be cherry-picked to the 11 release branch (which contains D92759 so it won't 
build with GCC 5.x )

Thanks!
- Raphael

> On 25 Jan 2021, at 10:04, Sylvestre Ledru via Phabricator 
>  wrote:
> 
> sylvestre.ledru added a comment.
> 
> This change doesn't build with gcc 5.3.1.
> More details: https://bugs.llvm.org/show_bug.cgi?id=48869
> 
> It would be great if you have could have a look! thanks
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D92759/new/
> 
> https://reviews.llvm.org/D92759
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 1b1d981 - Revert "Revert "Add the ability to write target stop-hooks using the ScriptInterpreter.""

2020-10-01 Thread RaphaelTeemperorIsemann via lldb-commits
+1, I have two machines with very similar setup where only the one that is 
under heavy load sees the test failures.

- Raphael

> On 1 Oct 2020, at 14:24, Pavel Labath  wrote:
> 
> On 30/09/2020 23:21, Jim Ingham wrote:
>> The test doesn’t seem to be flakey in the “run it a bunch of times and
>> it will eventually fail” type flakey.  I ran the test 200 times on my
>> machine and didn’t get a failure.
> 
> Actually, it seems like exactly the typical kind of flaky test to me --
> it mostly works when run on its own, but starts failing as soon as the
> system comes under load.
> 
> It didn't fail for me either for over 100 iterations. However, as soon
> as I cranked up the cpu load (compiling llvm is good at that), it failed
> almost immediately.
> 
> It also doesn't seem to be related to the way the stop hook resumes the
> process.
> 
> is one example where the auto_continue version of the test fails, and I
> have seen both tests fail on my machine.
> 
> I have some traces of failing and successful runs of the test (will send
> them to you in a private email). I didn't dive too deeply, but the
> problem does not seem to be related to python stop hooks. It looks more
> like a general stop hook bug, which we've had problems with in the past.
> 
> The problems seems to be that the process.Continue() on the main thread
> returns early, and so the subsequent checks (for the topmost frame etc.)
> execute concurrently with the "step out" action. In the "Failure" file
> I'll send you you can see that (line 9222) SBFrame::GetFunctionName is
> called before the inferior process stops in the main function (the
> processing of that happens immediately after that line, on the
> "intern-state" thread).
> 
> pl

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] f369d51 - [lldb] avoid assert in threadsanitizer tests on linux

2020-09-04 Thread RaphaelTeemperorIsemann via lldb-commits
I already fixed that bug on Monday, so this is now just assigning a second 
fallback value. See https://reviews.llvm.org/D86593 

Cheers,
- Raphael

> On 3 Sep 2020, at 21:18, Luboš Luňák via lldb-commits 
>  wrote:
> 
> 
> Author: Luboš Luňák
> Date: 2020-09-03T21:18:17+02:00
> New Revision: f369d51896e1c0f61df253b116c42771479549df
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/f369d51896e1c0f61df253b116c42771479549df
> DIFF: 
> https://github.com/llvm/llvm-project/commit/f369d51896e1c0f61df253b116c42771479549df.diff
> 
> LOG: [lldb] avoid assert in threadsanitizer tests on linux
> 
> The tests are unsupported on linux, but they assert in
> Thread::GetStopDescriptionRaw() because of empty stop reason
> description. And it is empty because
> InstrumentationRuntimeTSan::NotifyBreakpointHit() fails
> to get report from InstrumentationRuntimeTSan::RetrieveReportData(),
> which is possibly(?) the reason why this is unsupported on linux.
> Add a dummy stop reason description for this case, which changes
> the test result from failing to unsupported.
> 
> Added: 
> 
> 
> Modified: 
>
> lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
>  
> b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
> index a2954f556b10..68e732538158 100644
> --- 
> a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
> +++ 
> b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
> @@ -855,6 +855,8 @@ bool InstrumentationRuntimeTSan::NotifyBreakpointHit(
> });
> report->GetAsDictionary()->AddBooleanItem("all_addresses_are_same",
>   all_addresses_are_same);
> +  } else {
> +stop_reason_description = "unknown ThreadSanitizer stop reason";
>   }
> 
>   // Make sure this is the right process
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] a06c28d - Temporarily revert "[test] Exit with an error if no tests are run."

2020-08-04 Thread RaphaelTeemperorIsemann via lldb-commits
If it helps, all the failing tests are pexpect tests which are always disabled 
on Windows (like, they don't even exist from the test runners POV I believe). 
So I guess that's accidentially triggering that error.

- Raphael

> On 4 Aug 2020, at 03:39, Jordan Rupprecht via lldb-commits 
>  wrote:
> 
> 
> Author: Jordan Rupprecht
> Date: 2020-08-03T18:37:50-07:00
> New Revision: a06c28df3e8c85ceb665d3d9a1ebc2853dfd87a9
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/a06c28df3e8c85ceb665d3d9a1ebc2853dfd87a9
> DIFF: 
> https://github.com/llvm/llvm-project/commit/a06c28df3e8c85ceb665d3d9a1ebc2853dfd87a9.diff
> 
> LOG: Temporarily revert "[test] Exit with an error if no tests are run."
> 
> This reverts commit adb5c23f8c0d60eeec41dcbe21d1b26184e1c97d. It surprisingly 
> fails on a windows build bot: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/18009
> 
> Will reland after some investigation and/or after adding some extra logging 
> to help debug the issue.
> 
> Added: 
> 
> 
> Modified: 
>lldb/packages/Python/lldbsuite/test/dotest.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
> b/lldb/packages/Python/lldbsuite/test/dotest.py
> index 6607f52c49db..3fb802f1c1aa 100644
> --- a/lldb/packages/Python/lldbsuite/test/dotest.py
> +++ b/lldb/packages/Python/lldbsuite/test/dotest.py
> @@ -1039,10 +1039,6 @@ def run_suite():
> (configuration.suite.countTestCases(),
>  configuration.suite.countTestCases() != 1 and "s" or ""))
> 
> -if configuration.suite.countTestCases() == 0:
> -logging.error("did not discover any matching tests")
> -exitTestSuite(1)
> -
> # Invoke the test runner.
> if configuration.count == 1:
> result = unittest2.TextTestRunner(
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 0d5fc82 - [lldb] Eliminate unneeded value parameters in Utility (NFC)

2020-07-24 Thread RaphaelTeemperorIsemann via lldb-commits
We could make a custom attribute for types that should be always passed by 
value and then maybe have Clang handle that. The problem seems generic enough 
that maybe other people could find this useful.

- Raphael

> On 24 Jul 2020, at 09:27, Pavel Labath via lldb-commits 
>  wrote:
> 
> On 23/07/2020 18:24, Adrian Prantl wrote:
>> Is there some clever C++ way to prohibit taking an object's address (similar 
>> to how you can delete a copy constructor)?
>> 
>> -- adrian
> 
> You could =delete the address-of operator, but redefining unary & is
> generally frowned upon and I am not sure it will achieve what you want
> -- it would not prevent anyone from binding a reference to an existing
> object.
> 
> I don't think there's any way to prevent that, and even if it were, it
> would likely break too much generic code which expects to accept objects
> by const&.
> 
> pl
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 074b121 - Reland [lldb] Unify type name matching in FormattersContainer

2020-07-23 Thread RaphaelTeemperorIsemann via lldb-commits
That's actually a better idea. Relanded it with the member removed and 
constructor deleted. Thanks!

- Raphael

> On 23 Jul 2020, at 09:49, Eric Christopher  wrote:
> 
> Hi Raphael,
> 
> I've temporarily reverted this again with hopefully a better explanation here:
> 
> echristo@athyra ~/s/llvm-project> git push
> To github.com:llvm/llvm-project.git
>55c0f12a869..3a75466f41b  master -> master
> 
> One review thought: If you don't want people using the default constructor 
> for TypeMatcher perhaps just delete it? (i.e. = delete). If that's not where 
> you were going then perhaps we can come up with another way around this :)
> 
> Thanks and sorry for the inconvenience!
> 
> -eric
> 
> On Wed, Jul 22, 2020 at 12:34 AM Raphael Isemann via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> 
> Author: Raphael Isemann
> Date: 2020-07-22T09:32:28+02:00
> New Revision: 074b121642b286afb16adeebda5ec8236f7b8ea9
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9.diff
>  
> 
> 
> LOG: Reland [lldb] Unify type name matching in FormattersContainer
> 
> This was originally reverted because the Linux bots were red after this 
> landed,
> but it seems that was actually caused by a different commit. I double checked
> that this works on Linux, so let's reland this on Linux.
> 
> Summary:
> 
> FormattersContainer stores LLDB's formatters. It's implemented as a templated
> map-like data structures that supports any kind of value type and only allows
> ConstString and RegularExpression as the key types. The keys are used for
> matching type names (e.g., the ConstString key `std::vector` matches the type
> with the same name while RegularExpression keys match any type where the
> RegularExpression instance matches).
> 
> The fact that a single FormattersContainer can only match either by string
> comparison or regex matching (depending on the KeyType) causes us to always 
> have
> two FormatterContainer instances in all the formatting code. This also leads 
> to
> us having every type name matching logic in LLDB twice. For example,
> TypeCategory has to implement every method twice (one string matching one, one
> regex matching one).
> 
> This patch changes FormattersContainer to instead have a single `TypeMatcher`
> key that wraps the logic for string-based and regex-based type matching and is
> now the only possible KeyType for the FormattersContainer. This means that a
> single FormattersContainer can now match types with both regex and string
> comparison.
> 
> To summarize the changes in this patch:
> * Remove all the `*_Impl` methods from `FormattersContainer`
> * Instead call the FormatMap functions from `FormattersContainer` with a
>   `TypeMatcher` type that does the respective matching.
> * Replace `ConstString` with `TypeMatcher` in the few places that directly
>   interact with `FormattersContainer`.
> 
> I'm working on some follow up patches that I split up because they deserve 
> their
> own review:
> 
> * Unify FormatMap and FormattersContainer (they are nearly identical now).
> * Delete the duplicated half of all the type matching code that can now use 
> one
>   interface.
> * Propagate TypeMatcher through all the formatter code interfaces instead of
>   always offering two functions for everything.
> 
> There is one ugly design part that I couldn't get rid of yet and that is that 
> we
> have to support getting back the string used to construct a `TypeMatcher` 
> later
> on. The reason for this is that LLDB only supports referencing existing type
> matchers by just typing their respective input string again (without even
> supplying if it's a regex or not).
> 
> Reviewers: davide, mib
> 
> Reviewed By: mib
> 
> Subscribers: mgorny, JDevlieghere
> 
> Differential Revision: https://reviews.llvm.org/D84151 
> 
> 
> Added: 
> lldb/unittests/DataFormatter/FormattersContainerTest.cpp
> 
> Modified: 
> lldb/include/lldb/DataFormatters/DataVisualization.h
> lldb/include/lldb/DataFormatters/FormatManager.h
> lldb/include/lldb/DataFormatters/FormattersContainer.h
> lldb/include/lldb/DataFormatters/TypeCategory.h
> lldb/include/lldb/DataFormatters/TypeCategoryMap.h
> lldb/source/Commands/CommandObjectType.cpp
> lldb/source/DataFormatters/DataVisualization.cpp
> lldb/source/DataFormatters/FormatManager.cpp
> lldb/unittests/DataFormatter/CMakeLists.txt
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/include/lldb/DataFormatters/DataVisualization.h 
> b/lldb/include/lldb/DataFormatters/DataVisualization.h
> index 

Re: [Lldb-commits] [lldb] 90c1af1 - [lldb][NFC] Add more test for builtin formats

2020-06-21 Thread RaphaelTeemperorIsemann via lldb-commits
Sorry for that, it seems I overlooked those failure mails. Thanks!

> On 20 Jun 2020, at 23:22, Eric Christopher  wrote:
> 
> This is failing on some of the debian buildbots so I've temporarily reverted 
> it here:
> 
> commit 10b43541360efb35a1d33e9cf1e93023ebd69b15 (HEAD -> master, 
> origin/master, origin/HEAD)
> Author: Eric Christopher mailto:echri...@gmail.com>>
> Date:   Sat Jun 20 14:21:42 2020
> 
> Temporarily Revert "[lldb][NFC] Add more test for builtin formats"
> as it's failing on the debian buildbots:
> 
> http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/12531 
> 
> 
> This reverts commit 90c1af106a20785ffd01c0d6a41db8bc0160fd11.
> 
> buildbot link:
> 
> http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/12531 
> 
> 
> Sorry for any inconvenience.
> 
> -eric
> 
> On Sat, Jun 20, 2020 at 12:35 PM Raphael Isemann via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> 
> Author: Raphael Isemann
> Date: 2020-06-20T19:31:40+02:00
> New Revision: 90c1af106a20785ffd01c0d6a41db8bc0160fd11
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/90c1af106a20785ffd01c0d6a41db8bc0160fd11
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/90c1af106a20785ffd01c0d6a41db8bc0160fd11.diff
>  
> 
> 
> LOG: [lldb][NFC] Add more test for builtin formats
> 
> The previous tests apparently missed a few code branches in DumpDataExtractor
> code. Also renames the 'test_instruction' which had the same name as another
> test (and Python therefore ignored the test entirely).
> 
> Added: 
> 
> 
> Modified: 
> 
> lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
>  
> b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
> index 9fefae6bbf5c..1a413a13986a 100644
> --- 
> a/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
> +++ 
> b/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py
> @@ -42,6 +42,9 @@ def test(self):
>  self.assertIn("= 0\n", self.getFormatted("float", "0"))
>  self.assertIn("= 2\n", self.getFormatted("float", "0x4000"))
>  self.assertIn("= NaN\n", self.getFormatted("float", "-1"))
> +# Checks the float16 code.
> +self.assertIn("= 2\n", self.getFormatted("float", 
> "(__UINT16_TYPE__)0x4000"))
> +self.assertIn("= error: unsupported byte size (1) for float 
> format\n", self.getFormatted("float", "'a'"))
> 
>  # enumeration
>  self.assertIn("= 0\n", self.getFormatted("enumeration", "0"))
> @@ -59,6 +62,13 @@ def test(self):
> 
>  # octal
>  self.assertIn("= 04553207\n", self.getFormatted("octal", "1234567"))
> +self.assertIn("= 0221505317046536757\n", self.getFormatted("octal", 
> "(__uint128_t)0x123456789ABDEFull"))
> +
> +# complex float
> +self.assertIn("= error: unsupported byte size (1) for complex float 
> format\n", self.getFormatted("complex float", "'a'"))
> +
> +# complex integer
> +self.assertIn("= error: unsupported byte size (1) for complex 
> integer format\n", self.getFormatted("complex integer", "'a'"))
> 
>  # hex
>  self.assertIn("= 0x00abc123\n", self.getFormatted("hex", "0xABC123"))
> @@ -86,6 +96,17 @@ def test(self):
>  self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
> self.getFormatted("OSType", "cstring"))
>  self.assertIn('= " \\U001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
> self.getFormatted("unicode8", "cstring"))
> 
> +# FIXME: Passing a 'const char *' will ignore any given format,
> +# so we have to repeat the tests with a void* casts to actually test 
> our formats.
> +self.assertIn('= \\x9a\\x0f\\0\\0\\x01\\0\\0\\0\n', 
> self.getFormatted("character array", "(void *)cstring"))
> +self.assertIn('= \\x9a\\x0f\\0\\0\\x01\\0\\0\\0\n', 
> self.getFormatted("character", "(void *)cstring"))
> +self.assertIn('= " \\e\\a\\b\\f\\n\\r\\t\\vaA09"\n', 
> self.getFormatted("c-string", "(void *)cstring"))
> +# FIXME: Ignores the printables characters at the end.
> +self.assertIn('= \n', self.getFormatted("printable 
> character", "(void *)cstring"))
> +self.assertIn('= \'\\0\\0\\0\\x01\\0\\0\\x0f\\x9a\'\n', 
> self.getFormatted("OSType", "(void *)cstring"))
> +# FIXME: This should print a string.
> +self.assertIn('= 0x00010f9a\n', 

Re: [Lldb-commits] [PATCH] D80308: [lldb] Enable C++14 when evaluating expressions in a C++14 frame

2020-05-22 Thread RaphaelTeemperorIsemann via lldb-commits
Sorry, somehow I missed that mail. Reverted.

> On 22. May 2020, at 21:20, Max Kudryavtsev via Phabricator 
>  wrote:
> 
> max-kudr added a comment.
> 
> Hello,
> 
> I suppose this commit broke Windows LLDB tests 
> (http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486 -> 
> logs: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/16486/steps/test/logs/stdio).
>  Can you please fix or rollback it?
> 
> Thank you!
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D80308/new/
> 
> https://reviews.llvm.org/D80308
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] e98ef0a - [lldb] Fix several LLDB_LOGs with wrong indices in ClangASTSource.cpp

2020-03-04 Thread RaphaelTeemperorIsemann via lldb-commits
Yeah this whole thing is a disaster and it’s puzzling that we don’t test the 
whole logging but instead just crash on the end user machine. I’m currently 
writing a test that gives all this code at least coverage and then I guess we 
have to fix all of these bugs.

> On Mar 4, 2020, at 10:53 AM, Shafik Yaghmour  wrote:
> 
> This is a lot of errors, did it the index use to start at one before? 
> 
> There is a type below, noted inline
> 
>> On Mar 4, 2020, at 10:33 AM, Raphael Isemann via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2020-03-04T10:32:50-08:00
>> New Revision: e98ef0af2c725f12dc60556a039e947ddeeb9f42
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/e98ef0af2c725f12dc60556a039e947ddeeb9f42
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/e98ef0af2c725f12dc60556a039e947ddeeb9f42.diff
>> 
>> LOG: [lldb] Fix several LLDB_LOGs with wrong indices in ClangASTSource.cpp
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> index 4d98d9cbdda3..6a8c7bd46559 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> @@ -190,8 +190,8 @@ void ClangASTSource::CompleteType(TagDecl *tag_decl) {
>> 
>>  if (log) {
>>LLDB_LOG(log,
>> - "CompleteTagDecl on (ASTContext*){1} Completing "
>> - "(TagDecl*){2} named {3}",
>> + "CompleteTagDecl on (ASTContext*){0} Completing "
>> + "(TagDecl*){1} named {2}",
>> m_clang_ast_context->getDisplayName(), tag_decl,
>> tag_decl->getName());
>> 
>> @@ -220,7 +220,7 @@ void ClangASTSource::CompleteType(TagDecl *tag_decl) {
>>  ClangASTImporter::NamespaceMapSP namespace_map =
>>  m_ast_importer_sp->GetNamespaceMap(namespace_context);
>> 
>> -  LLDB_LOGV(log, "  CTD Inspecting namespace map{1} ({2} entries)",
>> +  LLDB_LOGV(log, "  CTD Inspecting namespace map{0} ({1} entries)",
>>namespace_map.get(), namespace_map->size());
>> 
>>  if (!namespace_map)
>> @@ -229,7 +229,7 @@ void ClangASTSource::CompleteType(TagDecl *tag_decl) {
>>  for (ClangASTImporter::NamespaceMap::iterator i = 
>> namespace_map->begin(),
>>e = namespace_map->end();
>>   i != e && !found; ++i) {
>> -LLDB_LOG(log, "  CTD Searching namespace {1} in module {2}",
>> +LLDB_LOG(log, "  CTD Searching namespace {0} in module {1}",
>> i->second.GetName(), i->first->GetFileSpec().GetFilename());
>> 
>>TypeList types;
>> @@ -478,12 +478,12 @@ void ClangASTSource::FindExternalLexicalDecls(
>>std::string ast_dump = ClangUtil::DumpDecl(decl);
>>if (const NamedDecl *context_named_decl =
>>dyn_cast(context_decl))
>> -  LLDB_LOG(log, "  FELD Adding [to {1}Decl {2}] lexical {3}Decl 
>> {4}",
>> +  LLDB_LOG(log, "  FELD Adding [to {0}Decl {1}] lexical {2}Decl 
>> 34}”,
> 
> 
> Typo should be {3} not 34}
> 
> 
>>   context_named_decl->getDeclKindName(),
>>   context_named_decl->getName(), decl->getDeclKindName(),
>>   ast_dump);
>>else
>> -  LLDB_LOG(log, "  FELD Adding lexical {1}Decl {2}",
>> +  LLDB_LOG(log, "  FELD Adding lexical {0}Decl {1}",
>>   decl->getDeclKindName(), ast_dump);
>>  }
>> 
>> @@ -527,19 +527,19 @@ void 
>> ClangASTSource::FindExternalVisibleDecls(NameSearchContext ) {
>>if (!context.m_decl_context)
>>  LLDB_LOG(log,
>>   "ClangASTSource::FindExternalVisibleDecls on "
>> -   "(ASTContext*){1} '{2}' for '{3}' in a NULL DeclContext",
>> +   "(ASTContext*){0} '{1}' for '{2}' in a NULL DeclContext",
>>   m_ast_context, m_clang_ast_context->getDisplayName(), name);
>>else if (const NamedDecl *context_named_decl =
>> dyn_cast(context.m_decl_context))
>>  LLDB_LOG(log,
>>   "ClangASTSource::FindExternalVisibleDecls on "
>> -   "(ASTContext*){1} '{2}' for '{3}' in '{4}'",
>> +   "(ASTContext*){0} '{1}' for '{2}' in '{3}'",
>>   m_ast_context, m_clang_ast_context->getDisplayName(), name,
>>   context_named_decl->getName());
>>else
>>  LLDB_LOG(log,
>>   "ClangASTSource::FindExternalVisibleDecls on "
>> -   "(ASTContext*){1} '{2}' for '{3}' in a '{4}'",
>> +   "(ASTContext*){0} '{1}' for '{2}' in a '{3}'",
>>   m_ast_context, m_clang_ast_context->getDisplayName(), name,
>>   

Re: [Lldb-commits] [lldb] b6b3fcd - [lldb] Don't iterate over a std::set in SymbolFileDWARF::GetTypes to make it deterministic

2020-03-02 Thread RaphaelTeemperorIsemann via lldb-commits
I was just grepping for unordered data structures (e.g. ’std::set<‘) that use 
pointers/pointer-like objects (e.g. CompilerType with its operator<) and then 
reading the related code. Not sure if there is a good way to detect this stuff 
automatically. I guess we could have had a Clang plugin that creates a warning 
when code iterates over an unordered data structure that has a pointer-like 
type as a key (probably would cause a bunch of false-positives but if someone 
ran this on his own machine from time to time that would be enough I think).

> On Mar 2, 2020, at 3:07 PM, Davide Italiano  wrote:
> 
> This is really good. How did you find it, Raphael?
> I generally use LLVM_REVERSE_ITERATOR but given this wasn’t a container in 
> llvm, I’m curious to hear.
> 
> —
> Davide
> 
>> On Mar 2, 2020, at 15:04, Raphael Isemann via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2020-03-02T15:03:45-08:00
>> New Revision: b6b3fcdcb8cdfb887e26d27bee03b997d2d65888
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/b6b3fcdcb8cdfb887e26d27bee03b997d2d65888
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/b6b3fcdcb8cdfb887e26d27bee03b997d2d65888.diff
>> 
>> LOG: [lldb] Don't iterate over a std::set in 
>> SymbolFileDWARF::GetTypes to make it deterministic
>> 
>> Summary:
>> Currently `SymbolFileDWARF::TypeSet` is a typedef to a `std::set`.
>> In `SymbolFileDWARF::GetTypes` we iterate over a TypeSet variable when 
>> finding
>> types so that logic is non-deterministic as it depends on the actual pointer 
>> address values.
>> 
>> This patch changes the `TypeSet` to a `llvm::UniqueVector` which always 
>> iterates in
>> the order in which we inserted the types into the list.
>> 
>> Reviewers: JDevlieghere, aprantl
>> 
>> Reviewed By: JDevlieghere
>> 
>> Subscribers: mgrang, abidh, lldb-commits
>> 
>> Tags: #lldb
>> 
>> Differential Revision: https://reviews.llvm.org/D75481
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>   lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
>> b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> index c89ccb5bf960..c27b5c4c3495 100644
>> --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> @@ -324,10 +324,8 @@ void SymbolFileDWARF::GetTypes(const DWARFDIE , 
>> dw_offset_t min_die_offset,
>>  if (add_type) {
>>const bool assert_not_being_parsed = true;
>>Type *type = ResolveTypeUID(die, assert_not_being_parsed);
>> -if (type) {
>> -  if (type_set.find(type) == type_set.end())
>> -type_set.insert(type);
>> -}
>> +if (type)
>> +  type_set.insert(type);
>>  }
>>}
>> 
>> 
>> diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h 
>> b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> index a3928c8c3dd4..479235c0d86f 100644
>> --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> @@ -12,11 +12,11 @@
>> #include 
>> #include 
>> #include 
>> -#include 
>> #include 
>> #include 
>> 
>> #include "llvm/ADT/DenseMap.h"
>> +#include "llvm/ADT/SetVector.h"
>> #include "llvm/Support/Threading.h"
>> 
>> #include "lldb/Core/UniqueCStringMap.h"
>> @@ -439,7 +439,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
>> 
>>  bool FixupAddress(lldb_private::Address );
>> 
>> -  typedef std::set TypeSet;
>> +  typedef llvm::SetVector TypeSet;
>> 
>>  void GetTypes(const DWARFDIE , dw_offset_t min_die_offset,
>>dw_offset_t max_die_offset, uint32_t type_mask,
>> 
>> 
>> 
>> ___
>> lldb-commits mailing list
>> lldb-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D74556: [lldb] Don't call CopyForBreakpoint from a Breakpoint's constructor

2020-02-18 Thread RaphaelTeemperorIsemann via lldb-commits
Thanks!

> On Feb 18, 2020, at 1:24 PM, Tatyana Krasnukha 
>  wrote:
> 
> The test was passing unexpectedly, I already removed XFAIL decorator.
> 
> -Original Message-
> From: Raphael “Teemperor” Isemann  
> Sent: Tuesday, February 18, 2020 3:20 PM
> To: reviews+d74556+public+411e87b2a9392...@reviews.llvm.org
> Cc: Tatyana Krasnukha ; Jonas Devlieghere 
> ; jing...@apple.com; lldb-commits@lists.llvm.org; 
> liburd1...@outlook.com; sani...@subpath.org; chi...@raincode.com
> Subject: Re: [PATCH] D74556: [lldb] Don't call CopyForBreakpoint from a 
> Breakpoint's constructor
> 
> I assume this is causing the test to fail on Windows? 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lab.llvm.org-3A8011_builders_lldb-2Dx64-2Dwindows-2Dninja_builds_13896=DwIFAg=DPL6_X_6JkXFx7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=QVHIvP2O76mhlXu3NHpC60dV9plTxqLsbwE_fgLAJZk=jk0NPJjYvMlZIFBH2uPt1fo6zA543ZKEeBlU9108M_M=
>  
> 
>> On 18. Feb 2020, at 12:26, Tatyana Krasnukha via Phabricator 
>>  wrote:
>> 
>> tatyana-krasnukha closed this revision.
>> tatyana-krasnukha added a comment.
>> 
>> Closed by commit 185ef697ef5c 
>> >  >
>> 
>> 
>> Repository:
>> rLLDB LLDB
>> 
>> CHANGES SINCE LAST ACTION
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74556_new_=DwIFAg=DPL6_X_6JkXFx7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=QVHIvP2O76mhlXu3NHpC60dV9plTxqLsbwE_fgLAJZk=wjRYSdktS4jTXskJlmyc_cPVY0vx5rODphb2Q-TWpRI=
>>  
>> 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74556=DwIFAg=DPL6_X_6JkXFx7AXWqB0tg=8NZfjV_ZLY_S7gZyQMq8mj7tiN4vlymPiSt0Wl0jegw=QVHIvP2O76mhlXu3NHpC60dV9plTxqLsbwE_fgLAJZk=eSiNRVypMo66Enyp_4OaR4YA1Bp_jSqCp92hhqUo3zI=
>>  
>> 
>> 
>> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D74556: [lldb] Don't call CopyForBreakpoint from a Breakpoint's constructor

2020-02-18 Thread RaphaelTeemperorIsemann via lldb-commits
I assume this is causing the test to fail on Windows? 
http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/13896

> On 18. Feb 2020, at 12:26, Tatyana Krasnukha via Phabricator 
>  wrote:
> 
> tatyana-krasnukha closed this revision.
> tatyana-krasnukha added a comment.
> 
> Closed by commit 185ef697ef5c 
> 
> 
> 
> Repository:
>  rLLDB LLDB
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D74556/new/
> 
> https://reviews.llvm.org/D74556
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D74475: [lldb] Replace assertTrue(a == b, "msg") with assertEquals(a, b, "msg") in the test suite

2020-02-13 Thread RaphaelTeemperorIsemann via lldb-commits
Thanks, I thought I fixed that before committing (that was the one change that 
Pavel missed IIRC).

> On 14. Feb 2020, at 07:53, Jonas Devlieghere via Phabricator 
>  wrote:
> 
> JDevlieghere added a comment.
> 
> Yup, looks like that was it, the bot is green again: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/13740
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D74475/new/
> 
> https://reviews.llvm.org/D74475
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] 8612e92 - [lldb][NFC] Remove GetASTContext call in ClangDeclVendor

2020-01-03 Thread RaphaelTeemperorIsemann via lldb-commits
We could use clang::Decl* etc. in these classes but that would defeat the whole 
concept that they are supposed to implement. CompilerType, CompilerDecl, etc.. 
are all classes that represent their respective equivalent in a language 
plugin. For Clang that is indeed clang::Type* and clang::Decl*, but Swift is 
storing swift::Type* and swift::Decl* in these void* pointers (and passes a 
SwiftASTContext* instead of a ClangASTContext* as a m_type_system). So if we 
change that to clang::Decl* then the all other language plugins like Swift are 
effectively broken. It would also mean that generic LLDB code using these 
classes would have a hard dependency on Clang.

And there shouldn’t be any place where we unconditionally cast this to 
clang::Decl* as this would directly crash swift-lldb. The m_type_system (which 
is either ClangASTContext or SwiftASTContext) can freely cast the void* to 
clang::Decl/swift::Decl as the checking happens through the virtual function 
call to the m_type_system interface (Swift types always have a SwiftASTContext 
as m_type_system, Clang types have a ClangASTContext as m_type_system).

Having said that, I’m not saying that the void* pointers in these classes are 
the best solution we could have here (there is a reason why I keep removing all 
these void* pointer conversions). If you see a way to get rid of them, then be 
my guest.

> On 3. Jan 2020, at 01:52, Shafik Yaghmour  wrote:
> 
> To further clarify all the member functions of CompilerDecl e.g.:
> 
> GetMangledName(…)
> GetDeclContext(…)
> 
> End up passing m_opaque_decl as an argument to a member function of 
> m_type_system and in these member functions they unconditionally cast the 
> argument to Decl* so why not make m_opaque_decl a Decl*
> 
>> On Jan 2, 2020, at 4:20 PM, Shafik Yaghmour via lldb-commits 
>>  wrote:
>> 
>> Apologies, m_opaque_decl member of CompilerDecl and operator < in 
>> CompilerDecl.h 
>> 
>>> On Jan 2, 2020, at 3:54 PM, Raphael “Teemperor” Isemann 
>>>  wrote:
>>> 
>>> Not sure if I can follow. What variable should just be a Decl* and what 
>>> operator>> 
>>> (Also yeah that reinterpret_cast could also be a static_cast)
>>> 
 On Jan 3, 2020, at 12:38 AM, Shafik Yaghmour  wrote:
 
 I am not crazy about the reinterpret_cast although if we look inside 
 CompilerDecl impl we can see that basically it is always assuming it is a 
 Decl* and just C-style casts it as such. So why not just make it a Decl*
 
 Also operator<(…) is super sketchy doing a < on void* 
 
> On Dec 28, 2019, at 6:52 AM, Raphael Isemann via lldb-commits 
>  wrote:
> 
> 
> Author: Raphael Isemann
> Date: 2019-12-28T15:20:19+01:00
> New Revision: 8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15.diff
> 
> LOG: [lldb][NFC] Remove GetASTContext call in ClangDeclVendor
> 
> Instead of returning NamedDecls and then calling GetASTContext
> to find back the ClangASTContext we used can just implement the
> FindDecl variant that returns CompilerDecls (and implement the
> other function by throwing away the ClangASTContext part of the
> compiler decl).
> 
> Added: 
> 
> 
> Modified: 
> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
> lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp 
> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> index c59722b7b4f8..0c5796650d45 100644
> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> @@ -15,16 +15,17 @@ using namespace lldb_private;
> 
> uint32_t ClangDeclVendor::FindDecls(ConstString name, bool append,
> uint32_t max_matches,
> -std::vector ) {
> +std::vector 
> ) {
> if (!append)
> decls.clear();
> 
> -  std::vector named_decls;
> -  uint32_t ret = FindDecls(name, /*append*/ false, max_matches, 
> named_decls);
> -  for (auto *named_decl : named_decls) {
> -decls.push_back(CompilerDecl(
> -ClangASTContext::GetASTContext(_decl->getASTContext()),
> -named_decl));
> +  

Re: [Lldb-commits] [lldb] 8612e92 - [lldb][NFC] Remove GetASTContext call in ClangDeclVendor

2020-01-02 Thread RaphaelTeemperorIsemann via lldb-commits
Not sure if I can follow. What variable should just be a Decl* and what 
operator On Jan 3, 2020, at 12:38 AM, Shafik Yaghmour  wrote:
> 
> I am not crazy about the reinterpret_cast although if we look inside 
> CompilerDecl impl we can see that basically it is always assuming it is a 
> Decl* and just C-style casts it as such. So why not just make it a Decl*
> 
> Also operator<(…) is super sketchy doing a < on void* 
> 
>> On Dec 28, 2019, at 6:52 AM, Raphael Isemann via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2019-12-28T15:20:19+01:00
>> New Revision: 8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15.diff
>> 
>> LOG: [lldb][NFC] Remove GetASTContext call in ClangDeclVendor
>> 
>> Instead of returning NamedDecls and then calling GetASTContext
>> to find back the ClangASTContext we used can just implement the
>> FindDecl variant that returns CompilerDecls (and implement the
>> other function by throwing away the ClangASTContext part of the
>> compiler decl).
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>   lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>   lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>   
>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>   
>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp 
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>> index c59722b7b4f8..0c5796650d45 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>> @@ -15,16 +15,17 @@ using namespace lldb_private;
>> 
>> uint32_t ClangDeclVendor::FindDecls(ConstString name, bool append,
>>uint32_t max_matches,
>> -std::vector ) {
>> +std::vector ) 
>> {
>>  if (!append)
>>decls.clear();
>> 
>> -  std::vector named_decls;
>> -  uint32_t ret = FindDecls(name, /*append*/ false, max_matches, 
>> named_decls);
>> -  for (auto *named_decl : named_decls) {
>> -decls.push_back(CompilerDecl(
>> -ClangASTContext::GetASTContext(_decl->getASTContext()),
>> -named_decl));
>> +  std::vector compiler_decls;
>> +  uint32_t ret = FindDecls(name, /*append*/ false, max_matches, 
>> compiler_decls);
>> +  for (CompilerDecl compiler_decl : compiler_decls) {
>> +clang::Decl *d =
>> +reinterpret_cast(compiler_decl.GetOpaqueDecl());
>> +clang::NamedDecl *nd = llvm::cast(d);
>> +decls.push_back(nd);
>>  }
>>  return ret;
>> }
>> 
>> diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h 
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>> index 85a10400a201..0c888de08841 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>> @@ -21,12 +21,10 @@ class ClangDeclVendor : public DeclVendor {
>> 
>>  virtual ~ClangDeclVendor() {}
>> 
>> -  uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
>> - std::vector ) override;
>> +  using DeclVendor::FindDecls;
>> 
>> -  virtual uint32_t FindDecls(ConstString name, bool append,
>> - uint32_t max_matches,
>> - std::vector ) = 0;
>> +  uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
>> + std::vector );
>> 
>>  static bool classof(const DeclVendor *vendor) {
>>return vendor->GetKind() >= eClangDeclVendor &&
>> 
>> diff  --git 
>> a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp 
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>> index ff0905dda4ef..0696c669f2e2 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>> @@ -80,7 +80,7 @@ class ClangModulesDeclVendorImpl : public 
>> ClangModulesDeclVendor {
>>Stream _stream) override;
>> 
>>  uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
>> - std::vector ) override;
>> + std::vector ) override;
>> 
>>  void ForEachMacro(const ModuleVector ,
>>std::function handler) 
>> override;
>> @@ -356,7 +356,7 @@ bool 
>> ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
>> uint32_t
>> 

Re: [Lldb-commits] [lldb] fe8e25a - [lldb][NFC] Create type-safe function for creating a CompilerType from a QualType

2020-01-02 Thread RaphaelTeemperorIsemann via lldb-commits
Somehow your mail is missing examples after the ‘… like:’ but the method is 
named similar to the GetType* methods directly below it. Don’t really have an 
opinion about whether this should be GetType* or GetCompilerType* (though I 
wouldn’t add the ‘As’ to it though as that’s usually used for casting 
utilities). I can rename the whole pack of function to GetCompilerType(…), that 
sounds good to me.

> On Jan 2, 2020, at 11:05 PM, Shafik Yaghmour  wrote:
> 
> Instead of GetType(...) I think GetCompilerType(…) is better or maybe even 
> GetAsCompilerType(…). It adds a bit more context, we have so many types. It 
> also feels a bit more consistent with methods like:
> 
> 
> 
>> On Jan 2, 2020, at 2:55 AM, Raphael Isemann via lldb-commits 
>>  wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2020-01-02T11:54:45+01:00
>> New Revision: fe8e25a48a2a0f8f508499ba950181dba3d600b0
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/fe8e25a48a2a0f8f508499ba950181dba3d600b0.diff
>> 
>> LOG: [lldb][NFC] Create type-safe function for creating a CompilerType from 
>> a QualType
>> 
>> LLDB frequently converts QualType to CompilerType. This is currently done 
>> like this:
>>   result = CompilerType(this, qual_type_var.getAsOpaquePtr())
>> There are a few shortcomings in this current approach:
>> 1. CompilerType's constructor takes a void* pointer so it isn't type safe.
>> 2. We can't add any sanity checks to the CompilerType constructor (e.g. that 
>> the type
>>actually belongs to the passed ClangASTContext) without expanding the 
>> TypeSystem API.
>> 3. The logic for converting QualType->CompilerType is spread out over all of 
>> LLDB so
>>changing it is difficult (e.g., what if we want to just pass the type ptr 
>> and not the
>>1type_ptr | qual_flags1 to CompilerType).
>> 
>> This patch adds a `ClangASTContext::GetType` function similar to the other 
>> GetTypeForDecl
>> functions that does this conversion in a type safe way.
>> 
>> It also adds a sanity check for Tag-based types that the type actually 
>> belongs to the
>> current ClangASTContext (Types don't seem to know their ASTContext, so we 
>> have to
>> workaround by looking at the decl for the underlying TagDecl. This doesn't 
>> cover all types
>> we construct but it's better than no sanity check).
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/include/lldb/Symbol/ClangASTContext.h
>>   lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>>   lldb/source/Plugins/Language/ObjC/NSArray.cpp
>>   
>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
>>   lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
>>   lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
>>   lldb/source/Symbol/ClangASTContext.cpp
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
>> b/lldb/include/lldb/Symbol/ClangASTContext.h
>> index e9a1d536ca8e..53ecd1bb78fc 100644
>> --- a/lldb/include/lldb/Symbol/ClangASTContext.h
>> +++ b/lldb/include/lldb/Symbol/ClangASTContext.h
>> @@ -164,6 +164,22 @@ class ClangASTContext : public TypeSystem {
>>  static bool AreTypesSame(CompilerType type1, CompilerType type2,
>>   bool ignore_qualifiers = false);
>> 
>> +  /// Creates a CompilerType form the given QualType with the current
>> +  /// ClangASTContext instance as the CompilerType's typesystem.
>> +  /// \param qt The QualType for a type that belongs to the ASTContext of 
>> this
>> +  ///   ClangASTContext.
>> +  /// \return The CompilerType representing the given QualType. If the
>> +  /// QualType's type pointer is a nullptr then the function 
>> returns an
>> +  /// invalid CompilerType.
>> +  CompilerType GetType(clang::QualType qt) {
>> +if (qt.getTypePtrOrNull() == nullptr)
>> +  return CompilerType();
>> +// Check that the type actually belongs to this ClangASTContext.
>> +assert(qt->getAsTagDecl() == nullptr ||
>> +   >getAsTagDecl()->getASTContext() == ());
>> +return CompilerType(this, qt.getAsOpaquePtr());
>> +  }
>> +
>>  CompilerType GetTypeForDecl(clang::NamedDecl *decl);
>> 
>>  CompilerType GetTypeForDecl(clang::TagDecl *decl);
>> 
>> diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> index ff86f9f818b2..b0043f9c0f64 100644
>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
>> @@ -1861,7 +1861,7 @@ CompilerType ClangASTSource::GuardedCopyType(const 
>> CompilerType _type) {
>>// seems to be generating bad types on occasion.
>>return CompilerType();
>> 
>> -  return CompilerType(m_clang_ast_context, 
>> 

Re: [Lldb-commits] [lldb] 22caa3c - [lldb] Add unit test for ClangASTImporter

2019-12-20 Thread RaphaelTeemperorIsemann via lldb-commits
Fixed by https://reviews.llvm.org/D71748. Will land this today.

> On 18. Dec 2019, at 23:03, Raphael “Teemperor” Isemann  
> wrote:
> 
> I’m actually already investigating that because I see the same failure when I 
> moved the ::Initialize and ::Terminate calls to SetUp and TearDown as part of 
> D71630. I’ll reply here when I have a fix, thanks!
> 
>> On Dec 18, 2019, at 10:54 PM, Jordan Rupprecht > > wrote:
>> 
>> We're seeing some odd test failures internally caused by this patch. For 
>> whatever reason (test ordering based on hashing, I guess), we're running the 
>> tests in a different order than upstream, and TestClangASTContext crashes 
>> when TestClangASTImporter runs first.
>> 
>> By default, it seems like TestClangASTContext happens to run first so the 
>> failure isn't usually seen, but the failure can be reproduced with 
>> --gtest_repeat=2
>> $ ninja SymbolTests && tools/lldb/unittests/Symbol/SymbolTests 
>> '--gtest_filter=TestClangAST*' --gtest_repeat=2
>> Repeating all tests (iteration 1) . . .
>> 
>> Note: Google Test filter = TestClangAST*
>> [[ TestClangASTContext passes ]]
>> [[ TestClangASTImporter passes ]]
>> 
>> Repeating all tests (iteration 2) . . .
>> 
>> Note: Google Test filter = TestClangAST*
>> [==] Running 21 tests from 2 test cases.
>> [--] Global test environment set-up.
>> [--] 13 tests from TestClangASTContext
>> [ RUN  ] TestClangASTContext.TestGetBasicTypeFromEnum
>> SymbolTests: /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:669: 
>> const clang::ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const: 
>> Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed.
>>  #0 0x0215e5a7 llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
>> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:548:11
>>  #1 0x0215e749 PrintStackTraceSignalHandler(void*) 
>> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:609:1
>>  #2 0x0215d02b llvm::sys::RunSignalHandlers() 
>> /src/llvm-project/llvm/lib/Support/Signals.cpp:67:5
>>  #3 0x0215eec5 SignalHandler(int) 
>> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:390:1
>>  #4 0x7f819b4523a0 __restore_rt 
>> (/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
>>  #5 0x7f819a3decfb gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
>>  #6 0x7f819a3c98ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
>>  #7 0x7f819a3c977f __tls_get_addr 
>> (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
>>  #8 0x7f819a3d7542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
>>  #9 0x020531d7 clang::QualType::getCommonPtr() const 
>> /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:0:5
>> #10 0x020529cc clang::QualType::getCanonicalType() const 
>> /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:6231:20
>> #11 0x02052879 clang::ASTContext::getCanonicalType(clang::QualType) 
>> const /src/llvm-project/llvm/../clang/include/clang/AST/ASTContext.h:2296:40
>> #12 0x02050960 clang::ASTContext::hasSameType(clang::QualType, 
>> clang::QualType) const 
>> /src/llvm-project/llvm/../clang/include/clang/AST/ASTContext.h:2312:12
>> #13 0x02047365 
>> TestClangASTContext_TestGetBasicTypeFromEnum_Test::TestBody() 
>> /src/llvm-project/lldb/unittests/Symbol/TestClangASTContext.cpp:57:3
>> <...>
>> 
>> Does the failure make sense to you?
>> No need to revert the patch -- we already have the test disabled internally, 
>> though we would like to re-enable it
>> 
>> On Mon, Dec 16, 2019 at 3:44 AM Raphael Isemann via lldb-commits 
>> mailto:lldb-commits@lists.llvm.org>> wrote:
>> 
>> Author: Raphael Isemann
>> Date: 2019-12-16T12:43:55+01:00
>> New Revision: 22caa3cfbcf5762a47acc40c425d9fe0c40da621
>> 
>> URL: 
>> https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621
>>  
>> 
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621.diff
>>  
>> 
>> 
>> LOG: [lldb] Add unit test for ClangASTImporter
>> 
>> Added: 
>> lldb/unittests/Symbol/TestClangASTImporter.cpp
>> 
>> Modified: 
>> lldb/unittests/Symbol/CMakeLists.txt
>> 
>> Removed: 
>> 
>> 
>> 
>> 
>> diff  --git a/lldb/unittests/Symbol/CMakeLists.txt 
>> b/lldb/unittests/Symbol/CMakeLists.txt
>> index aa86986f4e0e..02875b8b53c1 100644
>> --- a/lldb/unittests/Symbol/CMakeLists.txt
>> +++ b/lldb/unittests/Symbol/CMakeLists.txt
>> @@ -2,6 +2,7 @@ add_lldb_unittest(SymbolTests
>>LocateSymbolFileTest.cpp
>>PostfixExpressionTest.cpp
>>TestClangASTContext.cpp
>> +  TestClangASTImporter.cpp
>>TestDWARFCallFrameInfo.cpp
>>TestType.cpp
>>TestLineEntry.cpp
>> 
>> diff  --git 

Re: [Lldb-commits] [lldb] 22caa3c - [lldb] Add unit test for ClangASTImporter

2019-12-18 Thread RaphaelTeemperorIsemann via lldb-commits
I’m actually already investigating that because I see the same failure when I 
moved the ::Initialize and ::Terminate calls to SetUp and TearDown as part of 
D71630. I’ll reply here when I have a fix, thanks!

> On Dec 18, 2019, at 10:54 PM, Jordan Rupprecht  wrote:
> 
> We're seeing some odd test failures internally caused by this patch. For 
> whatever reason (test ordering based on hashing, I guess), we're running the 
> tests in a different order than upstream, and TestClangASTContext crashes 
> when TestClangASTImporter runs first.
> 
> By default, it seems like TestClangASTContext happens to run first so the 
> failure isn't usually seen, but the failure can be reproduced with 
> --gtest_repeat=2
> $ ninja SymbolTests && tools/lldb/unittests/Symbol/SymbolTests 
> '--gtest_filter=TestClangAST*' --gtest_repeat=2
> Repeating all tests (iteration 1) . . .
> 
> Note: Google Test filter = TestClangAST*
> [[ TestClangASTContext passes ]]
> [[ TestClangASTImporter passes ]]
> 
> Repeating all tests (iteration 2) . . .
> 
> Note: Google Test filter = TestClangAST*
> [==] Running 21 tests from 2 test cases.
> [--] Global test environment set-up.
> [--] 13 tests from TestClangASTContext
> [ RUN  ] TestClangASTContext.TestGetBasicTypeFromEnum
> SymbolTests: /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:669: 
> const clang::ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const: 
> Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed.
>  #0 0x0215e5a7 llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:548:11
>  #1 0x0215e749 PrintStackTraceSignalHandler(void*) 
> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:609:1
>  #2 0x0215d02b llvm::sys::RunSignalHandlers() 
> /src/llvm-project/llvm/lib/Support/Signals.cpp:67:5
>  #3 0x0215eec5 SignalHandler(int) 
> /src/llvm-project/llvm/lib/Support/Unix/Signals.inc:390:1
>  #4 0x7f819b4523a0 __restore_rt 
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
>  #5 0x7f819a3decfb gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
>  #6 0x7f819a3c98ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
>  #7 0x7f819a3c977f __tls_get_addr 
> (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
>  #8 0x7f819a3d7542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
>  #9 0x020531d7 clang::QualType::getCommonPtr() const 
> /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:0:5
> #10 0x020529cc clang::QualType::getCanonicalType() const 
> /src/llvm-project/llvm/../clang/include/clang/AST/Type.h:6231:20
> #11 0x02052879 clang::ASTContext::getCanonicalType(clang::QualType) 
> const /src/llvm-project/llvm/../clang/include/clang/AST/ASTContext.h:2296:40
> #12 0x02050960 clang::ASTContext::hasSameType(clang::QualType, 
> clang::QualType) const 
> /src/llvm-project/llvm/../clang/include/clang/AST/ASTContext.h:2312:12
> #13 0x02047365 
> TestClangASTContext_TestGetBasicTypeFromEnum_Test::TestBody() 
> /src/llvm-project/lldb/unittests/Symbol/TestClangASTContext.cpp:57:3
> <...>
> 
> Does the failure make sense to you?
> No need to revert the patch -- we already have the test disabled internally, 
> though we would like to re-enable it
> 
> On Mon, Dec 16, 2019 at 3:44 AM Raphael Isemann via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> 
> Author: Raphael Isemann
> Date: 2019-12-16T12:43:55+01:00
> New Revision: 22caa3cfbcf5762a47acc40c425d9fe0c40da621
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621
>  
> 
> DIFF: 
> https://github.com/llvm/llvm-project/commit/22caa3cfbcf5762a47acc40c425d9fe0c40da621.diff
>  
> 
> 
> LOG: [lldb] Add unit test for ClangASTImporter
> 
> Added: 
> lldb/unittests/Symbol/TestClangASTImporter.cpp
> 
> Modified: 
> lldb/unittests/Symbol/CMakeLists.txt
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/unittests/Symbol/CMakeLists.txt 
> b/lldb/unittests/Symbol/CMakeLists.txt
> index aa86986f4e0e..02875b8b53c1 100644
> --- a/lldb/unittests/Symbol/CMakeLists.txt
> +++ b/lldb/unittests/Symbol/CMakeLists.txt
> @@ -2,6 +2,7 @@ add_lldb_unittest(SymbolTests
>LocateSymbolFileTest.cpp
>PostfixExpressionTest.cpp
>TestClangASTContext.cpp
> +  TestClangASTImporter.cpp
>TestDWARFCallFrameInfo.cpp
>TestType.cpp
>TestLineEntry.cpp
> 
> diff  --git a/lldb/unittests/Symbol/TestClangASTImporter.cpp 
> b/lldb/unittests/Symbol/TestClangASTImporter.cpp
> new file mode 100644
> index ..17a0dfb6a348
> --- /dev/null
> +++ b/lldb/unittests/Symbol/TestClangASTImporter.cpp
> @@ -0,0 +1,220 @@
> +//===-- TestClangASTImporter.cpp 

Re: [Lldb-commits] [lldb] 7d175cf - [lldb] Xfail TestCallOverriddenMethod.py for aarch64/linux

2019-12-09 Thread RaphaelTeemperorIsemann via lldb-commits
Can you share the error output of the test failure?

> On 9. Dec 2019, at 12:38, Muhammad Omair Javaid via lldb-commits 
>  wrote:
> 
> 
> Author: Muhammad Omair Javaid
> Date: 2019-12-09T16:38:33+05:00
> New Revision: 7d175cf504bceb72a487a83ed9f640011832d46d
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/7d175cf504bceb72a487a83ed9f640011832d46d
> DIFF: 
> https://github.com/llvm/llvm-project/commit/7d175cf504bceb72a487a83ed9f640011832d46d.diff
> 
> LOG: [lldb] Xfail TestCallOverriddenMethod.py for aarch64/linux
> 
> This test still fails on Linux aarch64.
> Tested by buildbot running Ubuntu Bionic
> 
> Differential Revision: https://reviews.llvm.org/D70722
> 
> Added: 
> 
> 
> Modified: 
>
> lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
>  
> b/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
> index ddaafaab9fd2..2ad0c313d601 100644
> --- 
> a/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
> +++ 
> b/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
> @@ -63,6 +63,7 @@ def test_call_on_derived(self):
> # a vtable entry that does not exist in the compiled program).
> self.expect("expr d.foo()", substrs=["2"])
> 
> +@skipIf(oslist=["linux"], archs=["aarch64"])
> @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr43707")
> def test_call_on_temporary(self):
> """Test calls to overridden methods in derived classes."""
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [PATCH] D68597: Replace static const StringRef with StringRef (NFC)

2019-10-08 Thread RaphaelTeemperorIsemann via lldb-commits
We actually call the libc++abi guard functions even for const static 
StringRefs, so that patch actually saves us a lot.

> On Oct 7, 2019, at 11:43 PM, Adrian Prantl via Phabricator via lldb-commits 
>  wrote:
> 
> aprantl created this revision.
> aprantl added a reviewer: JDevlieghere.
> 
> I just don't think that we are saving anything by making these StringRefs 
> global variables. The strings they reference are constants anyway.
> 
> 
> https://reviews.llvm.org/D68597
> 
> Files:
>  lldb/source/Interpreter/OptionValueBoolean.cpp
>  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
>  
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
>  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r369652 - [lldb][NFC] Fix indentation in CommandObjectProcess

2019-08-22 Thread RaphaelTeemperorIsemann via lldb-commits
Fix is already committed as 369660, sorry for that.
- Raphael

> On Aug 22, 2019, at 5:28 PM, Alexander Kornienko via lldb-commits 
>  wrote:
> 
> This commit breaks buildbots: 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/8107 
> 
> On Thu, Aug 22, 2019 at 3:49 PM Raphael Isemann via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> Author: teemperor
> Date: Thu Aug 22 06:50:54 2019
> New Revision: 369652
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=369652=rev 
> 
> Log:
> [lldb][NFC] Fix indentation in CommandObjectProcess
> 
> Modified:
> lldb/trunk/source/Commands/CommandObjectProcess.cpp
> 
> Modified: lldb/trunk/source/Commands/CommandObjectProcess.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectProcess.cpp?rev=369652=369651=369652=diff
>  
> 
> ==
> --- lldb/trunk/source/Commands/CommandObjectProcess.cpp (original)
> +++ lldb/trunk/source/Commands/CommandObjectProcess.cpp Thu Aug 22 06:50:54 
> 2019
> @@ -348,15 +348,15 @@ public:
>  partial_name, FileSpec::Style::native);
>  match_info.SetNameMatchType(NameMatch::StartsWith);
>}
> -  platform_sp->FindProcesses(match_info, process_infos);
> -  const size_t num_matches = process_infos.GetSize();
> -  if (num_matches == 0)
> -return;
> -  for (size_t i = 0; i < num_matches; ++i) {
> -request.AddCompletion(
> -llvm::StringRef(process_infos.GetProcessNameAtIndex(i),
> -
> process_infos.GetProcessNameLengthAtIndex(i)));
> -  }
> +  platform_sp->FindProcesses(match_info, process_infos);
> +  const size_t num_matches = process_infos.GetSize();
> +  if (num_matches == 0)
> +return;
> +  for (size_t i = 0; i < num_matches; ++i) {
> +request.AddCompletion(
> +llvm::StringRef(process_infos.GetProcessNameAtIndex(i),
> +process_infos.GetProcessNameLengthAtIndex(i;
> +  }
>  }
> 
>  // Instance variables to hold the values for command options.
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r369506 - [lldb][NFC] Add tests for invalid command invocations

2019-08-21 Thread RaphaelTeemperorIsemann via lldb-commits
Thanks, just pushed a fix where I merge that into the dedicated apropos test!

> On 21. Aug 2019, at 16:16, Pavel Labath  wrote:
> 
> On 21/08/2019 11:15, Raphael Isemann via lldb-commits wrote:
>> Author: teemperor
>> Date: Wed Aug 21 02:15:44 2019
>> New Revision: 369506
>> URL: http://llvm.org/viewvc/llvm-project?rev=369506=rev
>> Log:
>> [lldb][NFC] Add tests for invalid command invocations
>> Added:
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/apropos/
>> 
>> lldb/trunk/packages/Python/lldbsuite/test/functionalities/apropos/TestApropos.py
> 
> It looks like there is already a test called TestApropos.py in 
> ./packages/Python/lldbsuite/test/help/TestApropos.py.
> 
> This makes lldb-dotest sad:
> $ python2 bin/lldb-dotest
> ...
> Traceback (most recent call last):
>  File "/home/pavelo/ll/lldb/test/dotest.py", line 7, in 
>lldbsuite.test.run_suite()
>  File "/home/pavelo/ll/lldb/packages/Python/lldbsuite/test/dotest.py", line 
> 1237, in run_suite
>visit('Test', dirpath, filenames)
>  File "/home/pavelo/ll/lldb/packages/Python/lldbsuite/test/dotest.py", line 
> 881, in visit
>raise Exception("Found multiple tests with the name %s" % name)
> Exception: Found multiple tests with the name TestApropos.py

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


Re: [Lldb-commits] [lldb] r367385 - [CompletionRequest] Remove unimplemented members.

2019-07-31 Thread RaphaelTeemperorIsemann via lldb-commits
Thanks! Wasn’t sure back then if I can just remove that weird thing

> On Jul 31, 2019, at 5:48 AM, Jonas Devlieghere via lldb-commits 
>  wrote:
> 
> Author: jdevlieghere
> Date: Tue Jul 30 20:48:29 2019
> New Revision: 367385
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=367385=rev
> Log:
> [CompletionRequest] Remove unimplemented members.
> 
> Completion requests have two fields that are essentially unimplemented:
> `m_match_start_point` and `m_max_return_elements`. This would've been
> okay, if it wasn't for the fact that this caused a bunch of useless
> parameters to be passed around. Occasionally there would be a comment or
> assert saying that they are not supported. This patch removes them.
> 
> Modified:
>lldb/trunk/include/lldb/Core/IOHandler.h
>lldb/trunk/include/lldb/Expression/REPL.h
>lldb/trunk/include/lldb/Host/Editline.h
>lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
>lldb/trunk/include/lldb/Utility/CompletionRequest.h
>lldb/trunk/source/API/SBCommandInterpreter.cpp
>lldb/trunk/source/Core/FormatEntity.cpp
>lldb/trunk/source/Core/IOHandler.cpp
>lldb/trunk/source/Expression/REPL.cpp
>lldb/trunk/source/Host/common/Editline.cpp
>lldb/trunk/source/Interpreter/CommandInterpreter.cpp
>lldb/trunk/source/Utility/CompletionRequest.cpp
>lldb/trunk/unittests/Utility/CompletionRequestTest.cpp
> 
> Modified: lldb/trunk/include/lldb/Core/IOHandler.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/IOHandler.h?rev=367385=367384=367385=diff
> ==
> --- lldb/trunk/include/lldb/Core/IOHandler.h (original)
> +++ lldb/trunk/include/lldb/Core/IOHandler.h Tue Jul 30 20:48:29 2019
> @@ -200,7 +200,6 @@ public:
> 
>   virtual int IOHandlerComplete(IOHandler _handler, const char 
> *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList , StringList 
> );
> 
>   virtual const char *IOHandlerGetFixIndentationCharacters() { return 
> nullptr; }
> @@ -417,7 +416,6 @@ private:
> 
>   static int AutoCompleteCallback(const char *current_line, const char 
> *cursor,
>   const char *last_char,
> -  int skip_first_n_matches, int max_matches,
>   StringList , StringList 
> ,
>   void *baton);
> #endif
> @@ -452,7 +450,6 @@ public:
> 
>   int IOHandlerComplete(IOHandler _handler, const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList , StringList ) 
> override;
> 
>   void IOHandlerInputComplete(IOHandler _handler,
> 
> Modified: lldb/trunk/include/lldb/Expression/REPL.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/REPL.h?rev=367385=367384=367385=diff
> ==
> --- lldb/trunk/include/lldb/Expression/REPL.h (original)
> +++ lldb/trunk/include/lldb/Expression/REPL.h Tue Jul 30 20:48:29 2019
> @@ -105,7 +105,6 @@ public:
> 
>   int IOHandlerComplete(IOHandler _handler, const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int max_matches,
> StringList , StringList ) 
> override;
> 
> protected:
> 
> Modified: lldb/trunk/include/lldb/Host/Editline.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Editline.h?rev=367385=367384=367385=diff
> ==
> --- lldb/trunk/include/lldb/Host/Editline.h (original)
> +++ lldb/trunk/include/lldb/Host/Editline.h Tue Jul 30 20:48:29 2019
> @@ -99,7 +99,6 @@ typedef int (*FixIndentationCallbackType
> 
> typedef int (*CompleteCallbackType)(const char *current_line,
> const char *cursor, const char *last_char,
> -int skip_first_n_matches, int 
> max_matches,
> StringList ,
> StringList , void *baton);
> 
> 
> Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=367385=367384=367385=diff
> ==
> --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original)
> +++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Tue Jul 30 
> 20:48:29 2019
> @@ -325,8 +325,7 @@ public:
>   //
>   // FIXME: Only max_return_elements == -1 is supported at 

Re: [Lldb-commits] [lldb] r364967 - [lldb] Mention automatic codesign setup script [NFC]

2019-07-02 Thread RaphaelTeemperorIsemann via lldb-commits
Yeah that whole document should probably be part of the build section for 
macOS. I can move the guide there because I anyway have to learn how the new 
website works :)

- Raphael

> On Jul 2, 2019, at 3:27 PM, Jonas Devlieghere  wrote:
> 
> Should we convert this to RST file and make it a proper part of the website?
> 
> On Tue, Jul 2, 2019 at 2:07 PM Raphael Isemann via lldb-commits 
> mailto:lldb-commits@lists.llvm.org>> wrote:
> Author: teemperor
> Date: Tue Jul  2 14:07:25 2019
> New Revision: 364967
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=364967=rev 
> 
> Log:
> [lldb] Mention automatic codesign setup script [NFC]
> 
> The script is the modern way of getting the certificate, so we should mention 
> it in
> the documentation.
> 
> Patch idea by Davidino Italiano!
> 
> Modified:
> lldb/trunk/docs/code-signing.txt
> 
> Modified: lldb/trunk/docs/code-signing.txt
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/code-signing.txt?rev=364967=364966=364967=diff
>  
> 
> ==
> --- lldb/trunk/docs/code-signing.txt (original)
> +++ lldb/trunk/docs/code-signing.txt Tue Jul  2 14:07:25 2019
> @@ -14,7 +14,10 @@ build folders that contained old signed
>  code signing using the executable's file system node, so you will need to
>  delete the file so the kernel clears its cache.
> 
> -If you don't have one yet you will need to:
> +Automatic setup:
> +- Run scripts/macos-setup-codesign.sh
> +
> +Manual setup steps:
>  - Launch /Applications/Utilities/Keychain Access.app
> 
>  - In Keychain Access select the "login" keychain in the "Keychains"
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org 
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits