rmaprath added inline comments.
================
Comment at: test/std/strings/basic.string/string.access/at.pass.cpp:41
+    const S& cs = s;
+    if (pos < cs.size())
+    {
----------------
rogfer01 wrote:
> rmaprath wrote:
> > For the cases where an exception //should've been// thrown, are we not 
> > entering the **undefined** domain at this point?
> > 
> > What if instead, we define two versions of the `test()` function? one 
> > containing the current code as-is, and the other only handles the cases 
> > where exceptions are not expected, and we modify the `main()` function 
> > below so that the correct `test()` case is invoked depending on the 
> > presence  / absence of exceptions? It's a bit more cumbersome than the 
> > current setup, but I'm not totally happy about treading into the undefined 
> > domain (if my understanding above is correct). 
> If I understand this test correctly, it checks for the `at` member function. 
> While certainly binding a const reference might throw, here it is bound to a 
> lvalue of the same type so no temporary construction should happen.
> 
> The original test checks both `s.size()` and `cs.size()`. Given that `size` 
> is a const member function it probably does not matter given that `cs` and 
> `s` are aliased, but see comment below.
Right, so it's the `at()` method which is expected to throw in this case. What 
you are doing here is carefully avoiding the exception throwing code-path while 
keeping the remaining assertions intact. Makes sense.

I'll go over the rest of the test cases to double check. But I'm happy with 
this.

(You need to wait for approval from @EricWF or @mclow.lists to commit)


https://reviews.llvm.org/D26136



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to