Page MenuHomePhabricator

Fix TestInlines.py on Windows
ClosedPublic

Authored by amccarth on Feb 26 2016, 11:03 AM.

Details

Summary

The inlining semantics for C and C++ are different, which affects the test's expectation of the number of times the function should appear in the binary. In the case of this test, C semantics means there should be three instances of inner_inline, while C++ semantics means there should be only two.

On Windows, clang uses C++ inline semantics even for C code, and there doesn't seem to be a combination of compiler flags to avoid this.

So, for consistency, I've recast the test to use C++ everywhere. Since the test resided under lang/c, it seemed appropriate to move it to lang/cpp.

This does not address the other XFAIL for this test on Linux/gcc. See https://llvm.org/bugs/show_bug.cgi?id=26710

Diff Detail

Event Timeline

amccarth updated this revision to Diff 49208.Feb 26 2016, 11:03 AM
amccarth retitled this revision from to Fix TestInlines.py on Windows.
amccarth updated this object.
amccarth added reviewers: zturner, spyffe.
amccarth added a subscriber: lldb-commits.
zturner edited edge metadata.Feb 26 2016, 11:16 AM

I don't know where we draw the line between test/lang and test/functionalities but I feel like the purpose of this test is just to make sure LLDB has the general ability to handle setting breakpoints on inline call sites. With that in mind, it make more sense to have this test in test/functionalities/inline-breakpoint, but I don't think it's a big deal either way.

packages/Python/lldbsuite/test/lang/cpp/inlines/inlines.h
1–4

Why do we need this header file now? It wasn't here before.

zturner added inline comments.Feb 26 2016, 11:18 AM
packages/Python/lldbsuite/test/lang/cpp/inlines/inlines.h
1–4

Oh wait I'm wrong. It was here before.

I'm going to interpret silence as consent. This review was mostly FYI.

This revision was automatically updated to reflect the committed changes.