[PATCH] D58535: [OptRemarks] Make OptRemarks more generic: rename OptRemarks to Remarks

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added inline comments.


================
Comment at: llvm/include/llvm-c/Remarks.h:34
 
-#define OPT_REMARKS_API_VERSION 0
+#define REMARKS_API_VERSION 0
 
----------------
JDevlieghere wrote:
> Are you sure this has no users? It's my understanding that we cannot break APIs in the C interface. 
This was never announced anywhere, so I believe it should be fairly safe to do the changes. @anemet, what do you think?

If this is not acceptable I'm happy to provide a compatibility layer.


================
Comment at: llvm/unittests/Remarks/RemarksParsingTest.cpp:278
+      "expected a value of scalar type."));
+  // Not a integer line in debug loc.
+  EXPECT_TRUE(parseExpectError(
----------------
paquette wrote:
> What happens if you give a negative line or column number?
For now, we get:

```
YAML:4:46: error: expected a value of integer type.

DebugLoc:        { File: test_parse.c, Line: -12, Column: 12 }
                                             ^~~
```

Which is pretty bad. I will update the parser in a future patch.


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

https://reviews.llvm.org/D58535