[clang] Introduce copy-on-write CompilerInvocation

[jansvoboda11]

This PR introduces new copy-on-write CompilerInvocation class (CowCompilerInvocation), which will be used by the dependency scanner to reduce the number of copies performed when generating command lines for discovered modules.

[benlangmuir]

I haven't dug into this in depth yet, but did you consider keeping the normal function names for the const versions and only rename the mutating ones? My thinking is you could use CowCompilerInvocation as a drop-in replacement for CompilerInvocation in immutable settings.

[jansvoboda11]

I did, but decided against it. There are places in Clang where we store the result of the const function in some kind of long-lived data structure, and someplace else modify the result of the non-const function, expecting the change to be reflected.

I wanted to make the semantic difference clear on both ends in case someone was inclined to swap CompilerInvocation for CowCompilerInvocation. I'm not sure this is the best way to prevent misuse of the API, so I'm open to alternatives.

Note that you can use CompilerInvocationBase to get the behavior you're describing. I use it to implement generateCC1CommandLine(), but it's supposed to be used sparingly. Maybe putting it into some kind of detail namespace would communicate this even better, but that pollutes the friend declarations.

[benlangmuir]

There are places in Clang where we store the result of the const function in some kind of long-lived data structure, and someplace else modify the result of the non-const function, expecting the change to be reflected.

I would be comfortable with this, because you would still have to call a distinctly named function (getMut...) in order to do the mutation itself IIUC. If they are const-casting of course that wouldn't be enough.

Note that you can use CompilerInvocationBase to get the behavior you're describing.

I think CompilerInvocationBase goes a step farther than what I described, because it would give you the mutating methods with the original names, no?

[jansvoboda11]

There are places in Clang where we store the result of the const function in some kind of long-lived data structure, and someplace else modify the result of the non-const function, expecting the change to be reflected.

I would be comfortable with this, because you would still have to call a distinctly named function (getMut...) in order to do the mutation itself IIUC. If they are const-casting of course that wouldn't be enough.

Ok, if you're fine with that, I'll use the original names for the const getters on CowCompilerInvocation.

Note that you can use CompilerInvocationBase to get the behavior you're describing.

I think CompilerInvocationBase goes a step farther than what I described, because it would give you the mutating methods with the original names, no?

No, CompilerInvocationBase doesn't have the mutating functions. RefBase and ValBase only expose the const getters. CompilerInvocationBase only adds the generate*() functions on top of those. The non-const getters are specific to CompilerInvocation and CowCompilerInvocation.

[jansvoboda11]

I evicted a couple of commits from this PR in order to work better with the new GitHub PR "squash and merge" workflow. I'll create new PRs for the remaining commits.

[shafik]

nit

[shafik]

Suggested change

	GenerateDiagnosticArgs(getDiagnosticOpts(), Consumer, false);
	GenerateDiagnosticArgs(getDiagnosticOpts(), Consumer, /*DefaultDiagColor=*/false);

[jansvoboda11]

Done in #65647.

[jansvoboda11]

Made the entirety of ValBase ref-counted in #65647, which also resolves some other comments here.

[benlangmuir]

This basically LGTM, but could we add some unit tests for the new type?