kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:583
TU.ExtraArgs.push_back("-std=c++17");
TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+ // Types might be different depending on the target triplet, we chose a
----------------
sammccall wrote:
> Is this redundant now? I think the default value here comes from the triple.
> (Maybe want to leave it so we have the most-trivial-possible-change for the
> branch, but add a FIXME)
right adding a fixme and sending a follow-up patch
================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:586
+ // fixed one to make sure tests passes on different platform.
+ TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
auto AST = TU.build();
----------------
sammccall wrote:
> this is a good fix for the branch.
> For trunk, should we do this in TestTU (i.e. for everything?)
it depends, as you've noted this will result in not testing some of the
platform dependent bits in other tests as well.
e.g. code completion, some code actions behaves differently on templates on
windows, we currently don't act on them(apart from putting necessary flags to
make behavior similar to what we care). But at least we notice them via
breakages in buildbots, if we put that into TestTU we might not notice such
differences until users report so.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73613/new/
https://reviews.llvm.org/D73613
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
