Question about changes to llvm::Argument::addAttr(AttributeSet AS) API

Hi,

I recently upgraded to the latest LLVM build and encountered a problem where the API for Argument::addAttr has changed.

Previously it was Argument::addAttr(Attribute A) and I was able to work with this.

The latest build has changed the method addAttr so that it requires an AttributeSet argument (Argument::addAttr(AttributeSet AS).

I’m not sure how to adjust to this change. The AttributeSet object seems to store an array of Attribute(s) to construct an AttributeSet argument for addAttr I need to know the index of the Argument in the function?

I can follow the lead of this code from Core.cpp:

void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
Argument *A = unwrap(Arg);
AttrBuilder B¶;
A->addAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1, B));
}

Is this all I need? What does the A->getArgNo()+1 provide? The index of the argument in the function argument list+1?

Is this change to the API described anywhere?

Best,

.Chris.

Hi,

I recently upgraded to the latest LLVM build and encountered a problem where the API for Argument::addAttr has changed.

Previously it was Argument::addAttr(Attribute A) and I was able to work with this.

The latest build has changed the method addAttr so that it requires an AttributeSet argument (Argument::addAttr(AttributeSet AS).

Yes.

I'm not sure how to adjust to this change. The AttributeSet object seems to store an array of Attribute(s) to construct an AttributeSet argument for addAttr I need to know the index of the Argument in the function?

That's correct.

I can follow the lead of this code from Core.cpp:

void LLVMAddAttribute(LLVMValueRef Arg, LLVMAttribute PA) {
  Argument *A = unwrap<Argument>(Arg);
  AttrBuilder B(PA);
  A->addAttr(AttributeSet::get(A->getContext(), A->getArgNo() + 1, B));
}

Is this all I need? What does the A->getArgNo()+1 provide? The index of the argument in the function argument list+1?

Yes. :slight_smile: It's the index of the argument into the list. The indices go like this:

Index number

However, we do list the thing we do change in the ReleaseNotes

Unfortunately this doesn't actually seem to be the case.

-- Sean Silva

No. It hasn't been written up. We typically don't do write-ups for API changes. However, we do list the thing we do change in the ReleaseNotes (these changes haven't made it there though).

The attributes API has undergone a horrendous amount of churn over the last few months, both before and after the 3.2 release. I've lost track of the number of times I've rewritten code interacting with this API over the past year and I now have a mess of #ifdefs just to support 3.1, 3.2 and trunk. Each time, the only warning I got was that my code stopped building with trunk, and the only way of knowing how to rewrite it was to read the attributes code.

It would be really great if whoever is responsible could step back from their code editor for a few minutes and write some documentation on how to migrate from each iteration to the next.

One of the goals of LLVM is to be a set of reusable libraries and this goal is not met by gratuitous API churn with no accompanying documentation. If we can't have a sane deprecation strategy or an automated migration tool then please can we at least have a token attempt at documentation? The AttributeSet documentation is just embarrassing. For example, the document for the class is:

This class manages the ref count for the opaque AttributeSetImpl
object and provides accessors for it

Great. It's a wrapper around an opaque type that isn't documented in the public headers. What is an AttributeSetImpl? What do I use it for? How does it relate to the last three classes that had similar functionality but different names? I can tell it uses the pImpl pattern by just looking at the class definition. That is not what the documentation should be telling me. The rest of this file is a case study in how not to write helpful documentation. This wouldn't be quite so bad if it were a class that's fairly obscure, but this is a class that is part of the core IR that pretty much every part of the pipeline needs to interact with. (And that's ignoring the fact that it is very confusing for an ordered collection that allows duplicates to be called a set).

David

Yeah. It's on my list. Give it time! :slight_smile:

-bw

No. It hasn't been written up. We typically don't do write-ups for API changes. However, we do list the thing we do change in the ReleaseNotes (these changes haven't made it there though).

The attributes API has undergone a horrendous amount of churn over the last few months, both before and after the 3.2 release. I've lost track of the number of times I've rewritten code interacting with this API over the past year and I now have a mess of #ifdefs just to support 3.1, 3.2 and trunk. Each time, the only warning I got was that my code stopped building with trunk, and the only way of knowing how to rewrite it was to read the attributes code.

I'm sorry about the problems. It's a continuing problem when working with a project where the APIs are not set in stone and are subject to change at a moment's notice.

It would be really great if whoever is responsible could step back from their code editor for a few minutes and write some documentation on how to migrate from each iteration to the next.

That would be more trouble than it's worth because the API is changing quite rapidly right now. A lot of it hasn't solidified just yet. Basically, once a migration doc was written it would be immediately out of date.

One of the goals of LLVM is to be a set of reusable libraries and this goal is not met by gratuitous API churn with no accompanying documentation. If we can't have a sane deprecation strategy or an automated migration tool then please can we at least have a token attempt at documentation? The AttributeSet documentation is just embarrassing. For example, the document for the class is:

This class manages the ref count for the opaque AttributeSetImpl
object and provides accessors for it

Great. It's a wrapper around an opaque type that isn't documented in the public headers. What is an AttributeSetImpl? What do I use it for? How does it relate to the last three classes that had similar functionality but different names?

It's an opaque object. It shouldn't concern people using the AttributeSet class. It's not documented in public headers for that very reason.

I can tell it uses the pImpl pattern by just looking at the class definition. That is not what the documentation should be telling me. The rest of this file is a case study in how not to write helpful documentation. This wouldn't be quite so bad if it were a class that's fairly obscure, but this is a class that is part of the core IR that pretty much every part of the pipeline needs to interact with. (And that's ignoring the fact that it is very confusing for an ordered collection that allows duplicates to be called a set).

Welcome to living on the top-of-tree! :slight_smile:

We are in between releases. It's expected that the APIs will be unstable. I wrote (several times) that the attributes were going to be rewritten. That implies a lot of things, including that everything about attributes and how you use them will change. Where applicable, there will be an auto-upgrade done. (One requirement is that the old bitcode files will continue to be parsed correctly (at least until release 4.0).) And yes the documentation is also in flux. It has been improving over time. The thing you quoted above is perhaps the worst comment from the Attributes.h file, and isn't indicative of the rest of the comments in there, which have been changing during the rewrite.

-bw

P.S. AttributeSets don't allow duplicates. :slight_smile:

Bill Wendling <wendling@apple.com> writes:

[snip]

Welcome to living on the top-of-tree! :slight_smile:

We are in between releases. It's expected that the APIs will be
unstable. [snip]

One thing is an unstable API, and a different thing is a code base where
an API change is introduced gradually. From the POV of users, the latter
is more like a pre-alpha stage API.

Using development branches would lessen the problem: you work on your
branch until you are happy with the API, then merge the changes into
trunk. When you are the only one working on the task, it is very easy to
do using git-svn.

As for living on the top-of-tree: with the development model of LLVM it
is desirable that as much users as possible work with ToT, because the
bug reports those users contribute. (And, what is the standard response
when somebody finds a bug on a released version? "upgrade to ToT") So
you don't want to scare away people from working with ToT.

Using a development branch and then slamming those changes into trunk is at odds with the llvm style incremental development philosophy. Living on ToT isn't easy. No one ever said it would be. The changes that are being complained about have only been happening for about a week now. And they're more stable day by day.

-bw

No. It hasn't been written up. We typically don't do write-ups for API changes. However, we do list the thing we do change in the ReleaseNotes (these changes haven't made it there though).

The attributes API has undergone a horrendous amount of churn over the last few months, both before and after the 3.2 release. I've lost track of the number of times I've rewritten code interacting with this API over the past year and I now have a mess of #ifdefs just to support 3.1, 3.2 and trunk. Each time, the only warning I got was that my code stopped building with trunk, and the only way of knowing how to rewrite it was to read the attributes code.

I'm sorry about the problems. It's a continuing problem when working with a project where the APIs are not set in stone and are subject to change at a moment's notice.

APIs in LLVM are not set in stone because we want to be able to evolve them as the requirements change. That should not be a license to constantly rewrite APIs with large numbers of consumers, but simply an indication that if the choice is between fixing the code and maintaining backwards compatibility then developers are free to fix the code. That doesn't mean that we should never think about the cost to downstream developers when we churn APIs, it just means that we shouldn't be held in a straitjacket by these concerns.

And the cost of changing an API is that it MUST be properly documented. Whenever you make the choice to break an API, you are saying 'I am saving my time by not writing compatibility interfaces, and everyone else must rewrite their code to support my changes.' If you then compound this by saying 'oh, and if you want to know how to rewrite your code, then read my code, my time is too valuable to waste documenting my work' then you do not encourage people to depend on LLVM.

It would be really great if whoever is responsible could step back from their code editor for a few minutes and write some documentation on how to migrate from each iteration to the next.

That would be more trouble than it's worth because the API is changing quite rapidly right now. A lot of it hasn't solidified just yet. Basically, once a migration doc was written it would be immediately out of date.

This comment is indicative of a code first, think second mentality. If the APIs are changing so quickly that even a high-level overview and broad migration guide would be out of date before it's finished then it seems pretty clear that the design stage was skipped entirely. While this is fine for code on the edges (e.g. a back-end under development, where changes don't affect any other users), it generates a lot of problems of people when it is code that is central to LLVM.

Effectively, here, you are saying that your time is far more valuable than everyone else's. You can, I hope, understand why this is not an opinion shared by those whose time you are spending to save some of your own. When you save 10 minutes of your time by not writing docs, you then make every user of this API (which means the author every out-of-tree front end, a lot of out-of-tree passes, and some back ends) spend at least twice that time each reading the code to try to figure out what the new equivalent of the old API is.

One of the goals of LLVM is to be a set of reusable libraries and this goal is not met by gratuitous API churn with no accompanying documentation. If we can't have a sane deprecation strategy or an automated migration tool then please can we at least have a token attempt at documentation? The AttributeSet documentation is just embarrassing. For example, the document for the class is:

This class manages the ref count for the opaque AttributeSetImpl
object and provides accessors for it

Great. It's a wrapper around an opaque type that isn't documented in the public headers. What is an AttributeSetImpl? What do I use it for? How does it relate to the last three classes that had similar functionality but different names?

It's an opaque object. It shouldn't concern people using the AttributeSet class. It's not documented in public headers for that very reason.

You are missing my point. The ONLY documentation describing AttributeSet tells me exactly one thing: that it is a wrapper around an AttributeSetImpl. If AttributeSetImpl it is an opaque object that users should not need to know about then why does the only public documentation for AttributeSet mention it at all? The documentation should tell me what an attribute set is (apparently, from looking over the code, it is an ordered collection of attributes indexed by parameter number?), not give me an implementation detail that apparently users shouldn't need to be aware of as the sole documentation.

I can tell it uses the pImpl pattern by just looking at the class definition. That is not what the documentation should be telling me. The rest of this file is a case study in how not to write helpful documentation. This wouldn't be quite so bad if it were a class that's fairly obscure, but this is a class that is part of the core IR that pretty much every part of the pipeline needs to interact with. (And that's ignoring the fact that it is very confusing for an ordered collection that allows duplicates to be called a set).

Welcome to living on the top-of-tree! :slight_smile:

Living on the top of tree is unavoidable for most LLVM downstream consumers. Since we have no deprecation policy, you either incrementally make changes following trunk, or you find that you need to make massive changes when a new release is branched. We encourage people to follow top of tree to minimise surprises and to give feedback on evolving APIs, so saying 'well, we're just going to churn the API's a lot, sucks to be you' to all of our downstream consumers is a very long way from ideal.

Take a look over the list archives for the last month for the number of questions from people using LLVM releases as old as 2.8 and you'll see what happens when we make it hard for people to follow trunk.

We are in between releases. It's expected that the APIs will be unstable. I wrote (several times) that the attributes were going to be rewritten. That implies a lot of things, including that everything about attributes and how you use them will change. Where applicable, there will be an auto-upgrade done. (One requirement is that the old bitcode files will continue to be parsed correctly (at least until release 4.0).) And yes the documentation is also in flux. It has been improving over time.

The APIs haven't just changed in trunk, they've changed with each release. I have some code that works with 3.1, 3.2 and trunk, and did work with 3.0 until I deleted that code to try to make it a bit cleaner. Every single one of these has very different APIs for accomplishing exactly the same thing. This is not rapid evolution to meet changing requirements, this is gratuitous API churn caused by coding first, rather than stopping to design the APIs properly in the first place.

Yes, it's great that you're rewriting the attributes interface now to be stable and extensible in the longer term, but the fact that this is now the fourth incompatible iteration of the APIs in as many releases implies that something is badly wrong with the design process.

The thing you quoted above is perhaps the worst comment from the Attributes.h file, and isn't indicative of the rest of the comments in there, which have been changing during the rewrite.

No indeed. Most of the methods are completely lacking comments and the file contains no explanation of the relationship between attributes, functions, parameters, and attribute sets.

-bw

P.S. AttributeSets don't allow duplicates. :slight_smile:

Really? In that case I'm using it wrongly. Which I suspected anyway, because it is almost completely undocumented. Are the indexes not parameter indexes? If not, then why are they exposed at all?

Using a development branch and then slamming those changes into trunk is at odds with the llvm style incremental development philosophy. Living on ToT isn't easy. No one ever said it would be. The changes that are being complained about have only been happening for about a week now. And they're more stable day by day.

No, they have been happening for at least a year. If you are honestly unaware of this, then I suggest that you stop hacking on the attributes until you're familiar with the various iterations that those APIs have gone through over the past two years.

David

Bill Wendling <wendling@apple.com> writes:

Using a development branch and then slamming those changes into trunk
is at odds with the llvm style incremental development philosophy.

In this case, the LLVM incremental style is counterproductive, for both
users and developers. Who is interested on watching your API going
through intermediate stages and (possibly) experimental points until it
stabilizes?

Living on ToT isn't easy. No one ever said it would be.

This is no reason for ignoring practices that can improve the
experience.

The changes that are being complained about have only been happening
for about a week now.

I have several modifications on my code base that go back until past
June for adapting to changes on the attributes API. But that's not the
point. I'm not thinking about your specific case, but on the general
one.

And they're more stable day by day.

You are proving my point. It is a waste to use trunk for things that are
considered by the author to be on work-on-progress state. That only
creates noise for both developers and users, without any advantage for
the author. Work on a feature branch and merge the changes when you are
happy with your code.

Bill Wendling <wendling@apple.com> writes:

Using a development branch and then slamming those changes into trunk
is at odds with the llvm style incremental development philosophy.

In this case, the LLVM incremental style is counterproductive, for both
users and developers. Who is interested on watching your API going
through intermediate stages and (possibly) experimental points until it
stabilizes?

People who are interested in catching errors quickly before they become expensive things to hunt down.

Living on ToT isn't easy. No one ever said it would be.

This is no reason for ignoring practices that can improve the
experience.

The most important thing is the health of the tree.

The changes that are being complained about have only been happening
for about a week now.

I have several modifications on my code base that go back until past
June for adapting to changes on the attributes API. But that's not the
point. I'm not thinking about your specific case, but on the general
one.

And they're more stable day by day.

You are proving my point. It is a waste to use trunk for things that are
considered by the author to be on work-on-progress state. That only
creates noise for both developers and users, without any advantage for
the author. Work on a feature branch and merge the changes when you are
happy with your code.

Are you new to LLVM or just haven't been paying attention? Developing features on trunk is what ALL developers do.

-bw

Bill Wendling <wendling@apple.com> writes:

In this case, the LLVM incremental style is counterproductive, for both
users and developers. Who is interested on watching your API going
through intermediate stages and (possibly) experimental points until it
stabilizes?

People who are interested in catching errors quickly before they
become expensive things to hunt down.

If I'm understanding what you say, you are interested on catching errors
on an experimental API by forcing it on everybody who works with ToT?
Hmmmm.

Living on ToT isn't easy. No one ever said it would be.

This is no reason for ignoring practices that can improve the
experience.

The most important thing is the health of the tree.

This seems to contradict what you wrote above. By going through several
iterations of the API development, you are putting the health of the
tree at risk multiple times, instead of just one if you merge to trunk
once you consider your API stable enough.

And they're more stable day by day.

You are proving my point. It is a waste to use trunk for things that are
considered by the author to be on work-on-progress state. That only
creates noise for both developers and users, without any advantage for
the author. Work on a feature branch and merge the changes when you are
happy with your code.

Are you new to LLVM or just haven't been paying attention? Developing
features on trunk is what ALL developers do.

First, please tone down. This is not about you personally, but about
blindly applying the LLVM development guidelines when doing something
else would be better for achieving the ultimate goal of such guidelines.
After all, the guidelines are just a set of practices for (hopefully)
obtaining an effect.

Second, not all developers use trunk for experimental code. More and
more of them use git or keep private patches until they consider that
the code is ready for prime time. I think that just a minority would
commit changes to stable core components of LLVM (much less alter an
API) as a means for storing the current state of their work-on-progress,
knowing that such changes will impact other developers and/or users.

Bill Wendling <wendling@apple.com> writes:

In this case, the LLVM incremental style is counterproductive, for both
users and developers. Who is interested on watching your API going
through intermediate stages and (possibly) experimental points until it
stabilizes?

People who are interested in catching errors quickly before they
become expensive things to hunt down.

If I'm understanding what you say, you are interested on catching errors
on an experimental API by forcing it on everybody who works with ToT?
Hmmmm.

It wasn't experimental! The rewrite was quite invasive and had to go through many different steps before it could settle. This would have happened whether I did it on a stupid git branch or not. Again, I could *not* slam a whole new set of 3000+ lines of code into the trunk without drawing the ire of everyone in the community.

Living on ToT isn't easy. No one ever said it would be.

This is no reason for ignoring practices that can improve the
experience.

The most important thing is the health of the tree.

This seems to contradict what you wrote above. By going through several
iterations of the API development, you are putting the health of the
tree at risk multiple times, instead of just one if you merge to trunk
once you consider your API stable enough.

No it doesn't contradict it. Each step had to be a solid change. If you are relying upon code that is changing and had been *informed* that it was changing, then you're doing it wrong.

And they're more stable day by day.

You are proving my point. It is a waste to use trunk for things that are
considered by the author to be on work-on-progress state. That only
creates noise for both developers and users, without any advantage for
the author. Work on a feature branch and merge the changes when you are
happy with your code.

Are you new to LLVM or just haven't been paying attention? Developing
features on trunk is what ALL developers do.

First, please tone down. This is not about you personally, but about
blindly applying the LLVM development guidelines when doing something
else would be better for achieving the ultimate goal of such guidelines.
After all, the guidelines are just a set of practices for (hopefully)
obtaining an effect.

You and Daniel have chosen me out of all of the changes going into the trunk as the whipping boy. This is personal. This conversation can serve no further purpose.

-bw

No. It hasn't been written up. We typically don't do write-ups for API changes. However, we do list the thing we do change in the ReleaseNotes (these changes haven't made it there though).

The attributes API has undergone a horrendous amount of churn over the last few months, both before and after the 3.2 release. I've lost track of the number of times I've rewritten code interacting with this API over the past year and I now have a mess of #ifdefs just to support 3.1, 3.2 and trunk. Each time, the only warning I got was that my code stopped building with trunk, and the only way of knowing how to rewrite it was to read the attributes code.

I'm sorry about the problems. It's a continuing problem when working with a project where the APIs are not set in stone and are subject to change at a moment's notice.

APIs in LLVM are not set in stone because we want to be able to evolve them as the requirements change. That should not be a license to constantly rewrite APIs with large numbers of consumers, but simply an indication that if the choice is between fixing the code and maintaining backwards compatibility then developers are free to fix the code. That doesn't mean that we should never think about the cost to downstream developers when we churn APIs, it just means that we shouldn't be held in a straitjacket by these concerns.

And the cost of changing an API is that it MUST be properly documented. Whenever you make the choice to break an API, you are saying 'I am saving my time by not writing compatibility interfaces, and everyone else must rewrite their code to support my changes.' If you then compound this by saying 'oh, and if you want to know how to rewrite your code, then read my code, my time is too valuable to waste documenting my work' then you do not encourage people to depend on LLVM.

You don't understand what I'm saying. The APIs were changing way too quickly for it to make sense to create such a document. I tried as best as I could to mitigate all of the problems, but there were several intermediate steps that had to happen before the attributes classes were in a proper state for the new feature work. The typical way to understand the APIs is to read the header files and/or look at existing code. The changes I made showed how to use the new APIs while I was going along.

It would be really great if whoever is responsible could step back from their code editor for a few minutes and write some documentation on how to migrate from each iteration to the next.

That would be more trouble than it's worth because the API is changing quite rapidly right now. A lot of it hasn't solidified just yet. Basically, once a migration doc was written it would be immediately out of date.

This comment is indicative of a code first, think second mentality. If the APIs are changing so quickly that even a high-level overview and broad migration guide would be out of date before it's finished then it seems pretty clear that the design stage was skipped entirely.

Not so. I had to change the existing attributes classes so that they a) still worked while I changed them, and b) would be in a state afterwards where they would support the new features we needed. The intermediate steps that were taken were necessary. They would have happened if I used git or cvs or any other vcs.

While this is fine for code on the edges (e.g. a back-end under development, where changes don't affect any other users), it generates a lot of problems of people when it is code that is central to LLVM.

Effectively, here, you are saying that your time is far more valuable than everyone else's.

Wrong. I'm saying that the document would be a waste of electrons because it would be out of date with the next change.

You can, I hope, understand why this is not an opinion shared by those whose time you are spending to save some of your own. When you save 10 minutes of your time by not writing docs, you then make every user of this API (which means the author every out-of-tree front end, a lot of out-of-tree passes, and some back ends) spend at least twice that time each reading the code to try to figure out what the new equivalent of the old API is.

Each step along the way had changes to existing API uses in the code base. If you wanted examples, those were them.

You don't understand what I'm saying. The APIs were changing way too quickly for it to make sense to create such a document. I tried as best as I could to mitigate all of the problems, but there were several intermediate steps that had to happen before the attributes classes were in a proper state for the new feature work. The typical way to understand the APIs is to read the header files and/or look at existing code. The changes I made showed how to use the new APIs while I was going along.

So the AttributeSet class is going away and being replaced with something else? If so, then why did it even make it into the tree. If not, then why is it not documented.

I'm sorry, but there is no excuse for committing large changes to a core bit of LLVM without even a brief overview of what the new class (which everyone is now expected to use) does. Who did the code review on this, because they really should have objected to the complete lack of documentation?

Not so. I had to change the existing attributes classes so that they a) still worked while I changed them, and b) would be in a state afterwards where they would support the new features we needed. The intermediate steps that were taken were necessary. They would have happened if I used git or cvs or any other vcs.

A comment saying 'this API is temporary for the migration between XXX and YYY' takes how long to write? Is your time really so valuable that this extra cost is too much?

The comment on other VCSs seems irrelevant, but if you are making such invasive changes that they must be done in multiple passes then either a feature branch and a merge or a local git clone seem the correct ways of doing them. Dumping 3000 lines of changes in the tree in one go is preferable to leaving the tree in flux for a week. You'll need the same code review in both cases, and it's much easier for people to do this when the end result is visible than to try to understand what all of the intermediate ones were for.

Wrong. I'm saying that the document would be a waste of electrons because it would be out of date with the next change.

I am starting to think that you either fundamentally misunderstand what documentation should exist. Each of the classes should have an overview saying what it is used for, and how it relates to other classes. If this is going to become wrong repeatedly, then the code should never make it to trunk, because it implies a complete lack of any kind of design work prior to committing.

If some of the individual methods need to change, then that's fine, but they can have a doc comment saying 'this API is currently under development and will change over the next week'. This saves downstream developers a lot of time - they can just avoid updating their code until it's a bit more stable, rather than having to .

Each step along the way had changes to existing API uses in the code base. If you wanted examples, those were them.

So, rather than you spending the 2 minutes required to write some quick documentation notes, every downstream LLVM consumer has to go to the header, run svn log to try to find the place where the old API was removed, then run svn diff on that revision to find out what the corresponding change was in, say, clang, and then try to map from that to their own code.

And you honestly think that this kind of behaviour is something that will encourage people to use LLVM?

David

You don't understand what I'm saying. The APIs were changing way too quickly for it to make sense to create such a document. I tried as best as I could to mitigate all of the problems, but there were several intermediate steps that had to happen before the attributes classes were in a proper state for the new feature work. The typical way to understand the APIs is to read the header files and/or look at existing code. The changes I made showed how to use the new APIs while I was going along.

So the AttributeSet class is going away and being replaced with something else? If so, then why did it even make it into the tree. If not, then why is it not documented.

The AttributeSet isn't going away. I'm not sure how you got that from what I wrote above.

I'm sorry, but there is no excuse for committing large changes to a core bit of LLVM without even a brief overview of what the new class (which everyone is now expected to use) does. Who did the code review on this, because they really should have objected to the complete lack of documentation?

Not so. I had to change the existing attributes classes so that they a) still worked while I changed them, and b) would be in a state afterwards where they would support the new features we needed. The intermediate steps that were taken were necessary. They would have happened if I used git or cvs or any other vcs.

A comment saying 'this API is temporary for the migration between XXX and YYY' takes how long to write? Is your time really so valuable that this extra cost is too much?

If you look at the svn log messages, I did just that for temporary classes and APIs. I also had several "FIXMEs" in the code with comments like that.

The comment on other VCSs seems irrelevant, but if you are making such invasive changes that they must be done in multiple passes then either a feature branch and a merge or a local git clone seem the correct ways of doing them. Dumping 3000 lines of changes in the tree in one go is preferable to leaving the tree in flux for a week.

Really? I got yelled at by Chris the last time I put several changes in the tree within a short amount of time. It also obviates the use of the buildbots to do something like that.

You'll need the same code review in both cases, and it's much easier for people to do this when the end result is visible than to try to understand what all of the intermediate ones were for.

Wrong. I'm saying that the document would be a waste of electrons because it would be out of date with the next change.

I am starting to think that you either fundamentally misunderstand what documentation should exist. Each of the classes should have an overview saying what it is used for, and how it relates to other classes. If this is going to become wrong repeatedly, then the code should never make it to trunk, because it implies a complete lack of any kind of design work prior to committing.

If some of the individual methods need to change, then that's fine, but they can have a doc comment saying 'this API is currently under development and will change over the next week'. This saves downstream developers a lot of time - they can just avoid updating their code until it's a bit more stable, rather than having to .

c.f. above, re log messages and "FIXMEs".

-bw

Oh, and the tree wasn't "in flux" in the manner you suggest. It was stable after each commit. Everything that was temporary was mentioned as such in the log messages and/or with FIXMEs. At any point in time, you or anyone else could have asked me what was up (even though I wrote that there would be sweeping changes well before they happened), and I would have gladly told you what was stable, what you had to change to make your code work, if you should wait, etc. Only two people did: the Mips and sanitizer people (and I suggested that they waited, or gave them workarounds which they could use in the meantime). I also fixed those projects I could (e.g. dragonegg). No one else made a sound while I did this. All of the rest I assumed didn't have problems or knew how to fix them by looking at the patches I submitted or would at the very least write to me and ask me how to fix their code. Again, no one (including you) ever did this.

The only thing that *might* have been missing was a write-up of exactly what the classes would eventually look like. However, that wouldn't have helped you during the transition. Now that the transition is over, you can now use the classes as you will. There may be small changes to them, but they won't be the same sweeping changes you are complaining about here.

I worked *very* hard to make things as stable as possible as quickly as possible. I didn't do anything "piecemeal" or "randomly". What happened happened because I needed things to be as stable as possible while I was changing something fundamental to LLVM.

And no, I will never "dump" 3000 lines of patches into the tree unless they were all purely mechanical in nature. I will prefer an incremental approach to that every time. If I did use git or whatever other vcs, I would have submitted the patches gradually, thus achieving the same result of modifying the tree gradually and it being "in flux" for a week.

-bw

Hi Bill,

  Just my 2 cents. How about sent a "HEADS UP" message to the ML
before you make changes. You can say "Hey, if you're using those
API I am going to change, please wait until I say everything is
stable." I think David can wait until you say the changes are done
and done, please sync with TOT.

  How about that, David?

Regards,
chenwj

Hi 陳韋任,

That’s a good idea and I missed doing that. I apologies for the problems my changes caused. The attribute classes are now stable enough to use. Any changes to them will be small enough for people to update their code easily.

-bw

Hi Bill,

That's a good idea and I missed doing that. I apologies for the problems my
changes caused. The attribute classes are now stable enough to use. Any changes
to them will be small enough for people to update their code easily.

  If I were you, I will send another mail saying class AttributeSet is
stable to ML. You know, this thread is too long to see this message. :wink:
Hope that doesn't bother you too much.

Cheers,
chenwj