This is an archive of the discontinued LLVM Phabricator instance.

[CUDA][HIP} Add a test for constexpr default ctor
ClosedPublic

Authored by yaxunl on Oct 9 2019, 6:12 PM.

Diff Detail

Event Timeline

yaxunl created this revision.Oct 9 2019, 6:12 PM
tra added inline comments.Oct 10 2019, 11:57 AM
test/SemaCUDA/constexpr-ctor.cu
15–28

Do we really need three identical templates? If they are needed to let compiler emit multiple diagnostics, perhaps we could just add another template parameter so we can get different instantiations.

yaxunl marked an inline comment as done.Oct 10 2019, 12:21 PM
yaxunl added inline comments.
test/SemaCUDA/constexpr-ctor.cu
15–28

the error is emitted on the default ctor of the template. If we do not use different templates, all errors are emitted on the same line, we cannot make sure some instantiations do not cause error and some instantiations should cause error.

Adding template parameter will not help, because the error will still be emitted to the same line.

tra added inline comments.Oct 10 2019, 1:57 PM
test/SemaCUDA/constexpr-ctor.cu
15–28

Having multiple errors attributed to the same source is OK. You can specify expected count. E.g.
dev-error 2 {{something}}. Single template appears to be sufficient (https://godbolt.org/z/G3WvYD).

You'll have different note diags emitted for individual errors, so checking them would both provide more info about that exactly the problem is and will verify that they are reported in the correct locations. Right now you are describing what/where triggers an error that as a comment. Letting compiler verify that instead would be better, IMO.

yaxunl marked 2 inline comments as done.Oct 10 2019, 3:22 PM
yaxunl added inline comments.
test/SemaCUDA/constexpr-ctor.cu
15–28

Yes I can use note. will do.

yaxunl updated this revision to Diff 224492.Oct 10 2019, 3:27 PM
yaxunl marked an inline comment as done.

revised by Artem's comments

tra accepted this revision.Oct 10 2019, 4:43 PM

LGTM.
Sometimes I wish it would be possible to specify some of the -verify diagnostics in temporal order, as opposed to tying them to locations. I.e. in this case it would be way more useful to see the call stack leading to the error at B::B. Oh, well.

This revision is now accepted and ready to land.Oct 10 2019, 4:43 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2019, 7:43 PM