v.g.vassilev added a comment.

In D126266#3629145 <https://reviews.llvm.org/D126266#3629145>, @vsapsai wrote:

> Hmm, I'm concerned with the pieces added purely for testing. Specifically, 
> `FileEntriesToReread` and TestHelper friend functions. Need to think how else 
> we can test it.

I am not thrilled about that either. We have discussed this approach here 
https://reviews.llvm.org/D126183 and we have past experience going that route. 
I would be happy to drop this test in favor of the mentioned clang-repl test. 
Should be enough to cover the usecase.

> Do you intend to support only the error cases like
>
>   clang-repl> #include "file_with_error.h"
>   // error is printed, we edit the file and include it again:
>   clang-repl> #include "file_with_error.h"
>
> or do you want to handle re-including files? Something like
>
>   clang-repl> #include "experiments.h"
>   // edit the file and include it again:
>   clang-repl> #include "experiments.h"
>
> Asking because maybe in the error case we commit to some state too eagerly 
> and fixing that sticky eagerness is another option (just guessing, have no 
> idea if it is an actual option and if it is "better").

We want both. The error case is somewhat easier to deal with. However, in case 
we have error in the logic in `#include "experiments.h"` we want to execute and 
then re-execute the new code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126266/new/

https://reviews.llvm.org/D126266

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

Reply via email to