AaronBallman wrote:

> > The Windows failures look annoying.
> > Taking one failure, Driver/aarch64-multilib-pauthabi.c. It contains:
> > `// CHECK: "-internal-externc-isystem" 
> > "{{.*}}/usr/include/aarch64-linux-pauthtest"`
> > However, on Windows, this has been canonicalized to 
> > `"C:\\_work\\llvm-project\\llvm-project\\clang\\test\\Driver\\Inputs\\multilib_aarch64_linux_tree\\usr\\include\\aarch64-linux-pauthtest"`
> >  (canonicalized path, from the specified sysroot 
> > `C:\\_work\\llvm-project\\llvm-project\\clang\\test\\Driver/Inputs/multilib_aarch64_linux_tree`).
> > So, fine, we can just change the CHECK line to use `{{(/|\\\\)}}`.
> > But taking another, Driver/aix-toolchain-include.cpp, it has: `// 
> > CHECK-INTERNAL-INCLUDE: "-internal-isystem" 
> > "[[RESOURCE_DIR]]{{(/|\\\\)}}include"` but, `"-resource-dir" is 
> > uncanoncalized, 
> > "C:\\_work\\llvm-project\\llvm-project\\clang\\test\\Driver/Inputs/resource_dir"`
> >  which can't match the now-canonicalized include path, 
> > `"C:\\_work\\llvm-project\\llvm-project\\clang\\test\\Driver\\Inputs\\resource_dir\\include"`.
> > Ugh.
> 
> Yeah, this is getting a bit messy...
> 
> > Notes about -no-canonical-prefixes :
> > > -no-canonical-prefixes instructs Clang to call realpath on the executable 
> > > name and use the dereferenced absolute path for the -cc1 command. This 
> > > path, either canonicalized by realpath or not, is used to derive the 
> > > resource directory. See 
> > > https://gcc.gnu.org/legacy-ml/gcc/2011-01/msg00429.html for some 
> > > background.
> > 
> > 
> > It's unclear whether -internal-isystem should reuse -no-canonical-prefixes. 
> > While I find `../..` for GCC/glibc paths difficult to read as well, they 
> > work best with all use cases. In addition, it is confusing that library 
> > paths are not canonicalized. I've tried this option with GCC, which does 
> > not change the `-v` output or `LIBRARY_PATH` in -### output.
> > ```
> > % /Dev/gcc/out/debug/gcc/xgcc -B ~/Dev/gcc/out/debug/gcc -c x.cc -v
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > I prefer that by default we don't do canonicalization.
> 
> We could make this a separate option, yeah. Also what with the Windows issues 
> (which I have a hard time debugging because I’m not on Windows...), it might 
> really be easier to just make it an opt-in flag at this point. It’s a bit sad 
> but it might be less of a hassle to do it that way...
> 
> CC @AaronBallman for opinions

I don't think an opt-in flag justifies the maintenance burden; I wouldn't 
expect users to enable that flag by default (honestly, the folks who benefit 
the most from the canonicalization are likely the same folks who would not 
think to add a new flag to the command line to get that canonicalization).

>From a Windows perspective, Clang's current behavior is mildly infuriating 
>because it mixes path separators and not all tools handle that well. But it is 
>existing behavior that we've lived with forever, so it's not the end of the 
>world if we continue to not canonicalize paths.

My weak preference is to consistently canonicalize all paths we want to display 
to the user on the belief that the canonical path is the most useful form for 
understanding what's being referenced. But if that's too disruptive to change, 
I can live with the status quo too.

https://github.com/llvm/llvm-project/pull/148745
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to