mdaniels added inline comments.
================
Comment at:
lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:5
+
+class StepIntoNamespace(TestBase):
+ mydir = TestBase.compute_mydir(__file__)
----------------
labath wrote:
> Am I correct in assuming that the "namespace" part here comes from the fact
> that global functions (e.g. `_Z3foov` aka `foo()`) would be found according
> to their base name (`foo`) or something?
>
> If yes, that's fine, but it would still be nice to mention (say in a comment
> or something) that the shared library is an important aspect of the test
> ("TestStepIntoNamespace" does not convey that notion).
>
> If this can also be reproduced with regular functions, then it'd be better to
> rename the test into something else (which includes the word "trampoline").
The lookup will be done with the full demangled name, so in this case it would
be "foo::foo()".
The namespace part was just because I had trouble reproducing when reducing the
testcase to a plain function call. Though I tried it again and can reproduce
the issue fine, so was likely user error on my part.
I will rename as suggested, thanks.
================
Comment at:
lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py:8
+
+ def test(self):
+ self.build()
----------------
labath wrote:
> mdaniels wrote:
> > clayborg wrote:
> > > Do we want to limit this to linux? I am not sure this will pass the
> > > windows buildbots if we don't restrict it
> > Looking at other tests that load a shared library, it seems I should have
> > at least
> >
> > ```
> > @skipIfRemote
> > @skipIfWindows
> > ```
> > But I can also just restrict it to the platforms I am able to test on
> > locally, linux and darwin, if there is still concern that other platforms
> > might fail.
> Neither of these is strictly necessary. Remote shared library tests need
> additional care as one has to copy the shared library to the remote system,
> but afaik, you're using the correct incantation which should do that
> automatically.
>
> Windows shared library support is not at the level of other platforms, but a
> simple setup like this should work. Or rather: if it fails, it will probably
> be due to the trampoline aspect, and not the shared library loading. So I
> think you don't need to add any decorators right now. We can add
> xfail-windows if it turns out to be failing.
Good to know, thanks for the feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127999/new/
https://reviews.llvm.org/D127999
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
