[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

Ben J via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 25 08:17:47 PDT 2023


Icantjuddle added a comment.

In D151145#4369406 <https://reviews.llvm.org/D151145#4369406>, @MyDeveloperDay wrote:

> We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-)



In D151145#4367495 <https://reviews.llvm.org/D151145#4367495>, @krasimir wrote:

> ...
> @Icantjuddle would you like to update this patch to fix the issue and enable your newly added tests?

Sounds like we don't really want this without a real fix; I'll give it a shot, hopefully won't be too bad. Thanks for the code pointer.

In D151145#4365580 <https://reviews.llvm.org/D151145#4365580>, @HazardyKnusperkeks wrote:

> As stated on Github I don't think we have a bug.

Are you sure, I'm pretty sure this is a bug? Can you elaborate on the discussion on github. I wouldn't want to fix something that isn't broken. If this is indeed expected maybe I can update the docs.



================
Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:92-99
     Style.RawStringFormats = {
         {
             /*Language=*/FormatStyle::LK_Cpp,
             /*Delimiters=*/{"cpp"},
             /*EnclosingFunctions=*/{},
             /*CanonicalDelimiter=*/"",
             BasedOnStyle,
----------------
HazardyKnusperkeks wrote:
> Couldn't you just set the style like you want, instead of parsing yaml?
Given that the problem seems to be with how the configs are parsed/processed, I wanted the repro to be most faithful to the user facing issue.


================
Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:177
+            format(R"test(
+t = R"pb(item:1)pb";)test",
+                   getRawStringPbStyleSeparateSection()));
----------------
HazardyKnusperkeks wrote:
> EXPECT_EQ
I used the function instead of the macro to conform to the convention of every other test in the file. 

The function provides its own justification.
```
  // Gcc 4.8 doesn't support raw string literals in macros, which breaks some
  // build bots. We use this function instead.
  void expect_eq(const std::string Expected, const std::string Actual) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151145



More information about the cfe-commits mailing list