This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Enable existing source_location tests
ClosedPublic

Authored by tbaeder on Jul 22 2023, 11:56 PM.

Details

Summary

As requested.

This depends on a few other patches though.

Diff Detail

Event Timeline

tbaeder created this revision.Jul 22 2023, 11:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2023, 11:56 PM
tbaeder requested review of this revision.Jul 22 2023, 11:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2023, 11:56 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tbaeder added inline comments.Jul 22 2023, 11:57 PM
clang/test/SemaCXX/source_location.cpp
764–768

The comment block just before this #if seems to say that the new interpreter is correct here...?

cor3ntin accepted this revision.Jul 23 2023, 1:35 AM
cor3ntin added inline comments.
clang/test/SemaCXX/source_location.cpp
764–768

Yes, which is interesting. Maybe modifying the existing interpreter properly would be doable then? I'll have to look into that...
I'm surprised it's only an evaluation issue

This revision is now accepted and ready to land.Jul 23 2023, 1:35 AM
This revision was landed with ongoing or failed builds.Sep 6 2023, 6:06 AM
This revision was automatically updated to reflect the committed changes.

I was rebasing D155064 locally and saw test failures for the new interpreter in this file.
The cause seems to me that the new interpreter is not handling some cases of SourceLocExpr correctly when they appear inside constant-evaluated or immediate-function context.
e.g.

struct A {
  int n = __builtin_LINE();
};
struct B {
  A a = {};
};
#line 100
consteval void f() {
  constexpr B c = {};
  static_assert(c.a.n == 101, "");
}

Live demo: https://godbolt.org/z/9Y7bzj56G

D155064 pushes constant-evaluated context against initializers of constexpr variables, so this bug appears in the test file.
I'm not in any hurry to push D155064 upstream, but if it might take several weeks for this bug to be fixed, I'd like this change to be reverted until it gets fixed.

@tbaeder Now it's working correctly. Thanks!