[PATCH] D151833: Respect "-fdiagnostics-absolute-paths" on emit include location

Charalampos Mitrodimas via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 3 03:14:37 PDT 2023


charmitro added inline comments.


================
Comment at: clang/test/Frontend/absolute-paths.c:6
+// NORMAL: In file included from [[ROOT_ABSOLUTE]]:4:
+// ABSOLUTE: In file included from [[ROOT_ABSOLUTE]]:4:
 
----------------
tbaeder wrote:
> charmitro wrote:
> > tbaeder wrote:
> > > charmitro wrote:
> > > > tbaeder wrote:
> > > > > This checks the same thing in both cases, but in the `NORMAL` case, it should //not// use the absolute path, shouldn't it?
> > > > You're correct. I was constantly testing it from the same path.
> > > > 
> > > > Since the relative path is going to be different depending on where you running from, would it be wise to accept a regular expression of any string in the `NORMAL` case?
> > > > 
> > > > For example, 
> > > > ```
> > > > NORMAL: In file included from {{.*}}:
> > > > ```
> > > I wonder if it would make sense to just check for the filename in the `NORMAL` case and then do a `NORMAL-NOT: [[ROOT_ABSOLUTE]]`?
> > Yes, that way we are testing if the warning contains the correct filename+LOC.
> > 
> > Something like: `// NORMAL: In file included from {{.*absolute-paths.c:4}}:`.
> > 
> > But why changefrom `ABSOLUTE:` to `NORMAL-NOT`?
> Your regex checks if the path ends with the file name, right? That would be true in both cases.
What else do you propose here?

Let me explain why this was the only obvious option for me.

In the case of `NORMAL`, it checks for the relative path based on the location you invoke the test commands. 

Given that, if we, for example,
1. run `llvm-lit` from the project root, the `NORMAL` case expects:
```
In file included from clang/test/Frontend/absolute-paths.c:4:
```
2. run `llvm-lit` from `$PROJECT_ROOT/clang/test/`, the `NORMAL` case expects:
```
In file included from Frontend/absolute-paths.c:4:
```

It seems unwise to me to hard-code the path based on what the CI excepts in the `NORMAL` scenario, because that way, whoever runs the test cases manually, will experience test failure.


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

https://reviews.llvm.org/D151833



More information about the cfe-commits mailing list