rogfer01 added inline comments.

================
Comment at: test/std/strings/basic.string/string.access/at.pass.cpp:41
+    const S& cs = s;
+    if (pos < cs.size())
+    {
----------------
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.


================
Comment at: test/std/strings/basic.string/string.access/at.pass.cpp:45
+        assert(cs.at(pos) == cs[pos]);
+        assert(pos < cs.size());
+    }
----------------
This is redundant, maybe I should change the if condition to be `if (pos < 
s.size())`


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